[PATCH] dirstate: add/improve method docstrings and some comments

Adrian Buehlmann adrian at cadifra.com
Mon Sep 28 02:43:28 CDT 2009


On 28.09.2009 04:01, Greg Ward wrote:
> On Sun, Sep 27, 2009 at 5:53 AM, Adrian Buehlmann <adrian at cadifra.com> wrote:
>> I have come to the conclusion that the granularity setting
>> is pointless up to the level that it starts harming. It doesn't
>> harm program execution but it does harm understanding of the code.
> 
> Oh good.  I realized the second part (harms understanding) several
> days ago, and strongly suspected the first part.
> 
>> It should be removed. We simply don't need it, it complicates
>> matters for no good reason.
> 
> +1
> 
>> I'm proposing the following change, which also introduces considerable
>> amount of new comment:
> 
> Nearly perfect.  Finally I can contribute something other than
> questions: my expertise as a native English speaker!
> 
>> +        # taking the modification time of the newly created temporary file as
>> +        # the filesystem's notion of 'now'
>> +        now = util.fstat(st).st_mtime
> 
> "Use the modification time of the newly created temporary file as
> the filesystem's notion of 'now'."
> 
>> +            if e[0] == 'n' and e[3] == now:
>> +                # The file was modified at "the same moment" (e.g. within the
>> +                # same second for filesystems like ext3) as we write dirstate.
> 
> "The file was modified "simultaneously" (e.g. within the same second for
> filesystems with 1 sec granularity) with the current write to dirstate."
> 
>> +                # This commonly happens for at least a couple of files on
>> +                # update. The user could change the file without changing its
>> +                # size immediately after the close of the transaction but still
>> +                # within the same second.
> 
> "... immediately after closing the transaction ..."
> 
>> +                # Avoid to mistreat such files as clean in future status calls
>> +                # by invalidating the file's stat data in dirstate, forcing a
>> +                # file compare on future status calls.
> 
> "Invalidate the file's stat data in dirstate, forcing future 'status'
> calls to compare file contents.  This prevents mistakenly treating
> such files as clean."
> 
> This will greatly improve the understandability of dirstate.  Thank you!

Ok. Thank you for reading the code and asking good questions.

I'll include your comment changes (without the double spaces after period, if
you don't mind) and offer to

* create a complete patch
* apply it to crew tip
* run the testsuite on FreeBSD with it
* do some additional manual testing, especially on Windows. That is, test it on FAT
  to confirm it works as expected for the two second resolution case.
* send it to the list for final review

_provided_ that at least someone of crew or Matt tells me that such a patch
has good chances for inclusion (with that unusual amount of comment).

Optionally, I could send the patch to the list before I start my thorough
final testing.

I don't want to do the whole testing dance in vain (did that enough times already :).
After all, this time the program is correct as is.


More information about the Mercurial-devel mailing list