Re: [PATCH] Implement the sync libnetfs stubs.

[olafBuddenhagen]

Hi,

On Tue, Aug 04, 2009 at 12:30:30AM +0200, Thomas Schwinge wrote:

> Please, please, please, let's try to finally get some of these patches
> installed before discussing matters to death.

The real problem is not patches being discussed to death, but rather
patches that have been reviewed still not getting comitted... Like some
of Zheng Da's patches for example.

I know that my reviews are too pedantic at times -- partially on
purpose, trying to establish some good practices; partially because of
my annoying perfectionism. But here I didn't just talk about formalities
like variable names or comments, but raised a fundamental concern!

The ironic thing here is that once you actually started looking at this
patch, you did the same kind of "discussing to death" :-)

> but I'm totally losing track of all these emails and huge discussion
> threads and proposals and misunderstandings and clarifications of them
> and further possibilities.

Don't worry -- I keep track of the patches I reviewed.

Better worry about those I didn't review... (Usually because they were
put only in the patch tracker, not sent to the list.)

> So.  Sergiu, if you need specific review of some parts of this patch
> (or any other patches), then please say so, otherwise please get it
> installed.

I'd be very grateful for you not to tell people to ignore my remarks and
commit broken patches.

The origianl patch is wrong and needs to be reverted, and a new one
comitted once all concerns actually have been addressed.

> I assume that you can confirm in some way (testing, staring at the
> code, ...) that it does the correct thing.  And should there be any
> breakage, and we discover it later on, we'll repair it later on.

The correct functioning of filesystem syncing is very hard to test; and
any errors will be extremely hard to track down later on. Which makes it
all the more important that we be able to say with some confidence that
the code is really doing the right thing.

-antrik-