Re: [PATCH] Simplify and fix --first-parent implementation

"Stephen R. van den Berg" <srb@cuci.nl> writes:

No reaction means just that. I either haven't looked at them, or after having looked at them I did not find them interesting enough to comment on.

The latter does not mean they are rejected, though. Remember, my review is NOT the only thing that counts here, other people's review and comment too here, and it tends to take time.

The proposed commit log message would express why the change was needed. It would present a use case that is useful (and argue why that use case is worth supporting), that is not easily supported with the existing code, and how the patch makes it so. When I review a patch posted on the list, here are the things I consider. This is pretty much personal, and other people may do things in different order:

 (0) The merit of the patch itself is not obvious from the diff, but there
     is no explanation; or

 (1) The argument to support the use case may not be convincing or may be
     outright wrong; or

 (2) Even if the argument is convincing, the claim that the current code
     does not support it may be false; or

 (3) The patch may not be the right way to support it and there may be
     better ways; or

 (4) The patch may make the new use case supported, but breaks existing
     use cases.

I look at (0) to _guess_ why the submitter thought the patch was a good idea when I have nothing better to do (but that seldom happens these days) or the submitter is new to the list.

Patches in (1) and (2) categories may get comments on what the patch tries to achieve, and for that I do not have to look at the diff. I tend start to look at the diff for patches in categories (3) and (4).

The ideal case is obviously:

 (5) The merit of the patch itself is very clear and there is a good
     explanation in the commit log message.