Re: [PATCH 2/3] unpack-trees: fix path search bug in verify_absent

Hi,

On Sun, Jan 04, 2009 at 02:01:14AM -0800, Junio C Hamano wrote:

    The function's purpose is ....  Before entering the loop to count the
    number of entries to skip, this check to detect if we do not even have
    to count appears.  When this check triggers, we know we do not want to
    skip anything, and returning constant 0 is much clearer than returning
    a variable cnt that was initialized to 0 near the beginning of the
    function; we haven't even started using it to count yet.

But the point is, if that is the reason the author thinks it is an
improvement, that probably needs to be stated.

If you want to check the validity of the patch you have to view it in
context anyways. Compared to understanding the change to the code, it takes
much longer to parse and understand the above paragraph _plus_ verify its
agreement with the code. I think you will agree that there is a limit to the
amount of documentation that's still useful.

My estimate of this limit is apparently much lower than what is expected by
the main contributors to this project. I respect that and I will try not to
waste your time any further.

What's sad, however, is that we are now discussing style and commenting
issues of a line of code, which, as by my analysis of [PATCH 3/3] never
actually gets executed in the first place. I would have been much more
curious about your comments on that.

Best regards,
Clemens