[PATCH 16 of 19] mq: remove redundant explicit invocations of savedirty

Mads Kiilerich mads at kiilerich.com
Fri Jan 13 04:45:33 CST 2012


On 01/13/2012 11:06 AM, Martin Geisler wrote:
> Mads Kiilerich<mads at kiilerich.com>  writes:
>
>> # HG changeset patch
>> # User Mads Kiilerich<mads at kiilerich.com>
>> # Date 1326245802 -3600
>> # Node ID 9ff0a0a1fe41ffec86acc76362b29c086151026d
>> # Parent  8bccb99f4ff21f335503798db047cf27c79cd77c
>> mq: remove redundant explicit invocations of savedirty
>
> Perhaps this could be folded with 15? When I read patch 15, I was
> surprised to only see the addition of lots of @mqlocksavedirty lines,
> without any deleted savedirty lines.

Yes, they (and many other patches) could be folded.

We could fold them before committing, but having them as separate 
checkpoints makes it easier to review and "prove" that they are good.

But more specifically:

Yes, the "beauty" of patch 15 is that it only adds code and a new 
property: It introduces a consistent locking scheme for mq operations 
and ensures that all modifying commands are consistent. That is very 
hard to observe and test, so one of the main points is that it only adds 
code that by definition shouldn't harm and in worst case is redundant 
... and that it do that without any regressions.

Patch 16 is then a more trivial clean-up that can be reviewed by its own 
merits. (It would be more controversial (and complex or slightly wrong) 
if it also removed the locks taken in the queue class.)

So: The split was intentional and I have a slight preference for doing 
it that way, but either way of doing it is fine with me.

/Mads


More information about the Mercurial-devel mailing list