Re: [PATCH 1/2] transport-helper: report errors properly

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 14, 2013, 05:23 UTC
Message-ID: <7v38ut7ilq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAMP44s3gOXvHknN1yXQcDYP=OBfjm7=eJnSkh5cj5QJNOarEWQ@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 16 quoted lines

> On Sat, Apr 13, 2013 at 1:00 AM, Jeff King <peff@peff.net> wrote:
>> On Sat, Apr 13, 2013 at 12:42:29AM -0500, Felipe Contreras wrote:
>>
>>> To me, the reality is obvious: my patch didn't require such a big
>>> commit message, the short version was fine, the only reason Jeff King
>>> insisted on a longer version is because the patch came from me.
>>
>> Get over yourself. The reason I suggested a longer commit message for
>> your commit is because after spending several hours figuring out what
>> the current code did, and what it should be doing instead, I wanted to
>> document that effort so that I and other readers did not have to do it
>> again later.
> ...
> The double standard might not come from you, perhaps you subject all
> the patches you review to the same standard, it comes from the fact
> that the patches you review have an unfair disadvantage.

There are reviewers who share the basic values [*1*] with I and the tradition of this project and whose judgement I can trust.

When somebody (like Peff) whose judgement I trust spends time to review a series, and writes his thought process to the degree that he thinks is appropriate, that's his judgement and I trust it.

You cannot expect perfect evenness from multiple people. For that matter, you cannot expect perfect evenness even from a single person, either.

When reviewing a proposed change to an area that I am intimately familiar with, I may immediately know some subtleties involved in a proposed solution without even reading the patch, and I may not even realize that such subtleties are hard to know without being somebody who are already familiar with that part of the codebase, and either in-code comment or in the log message may better spelled them out. On the other hand, when the change is in another area that I am not familiar with, I may request more explanation, if only for me to understand the issue.

I try to avoid the "I may know too well" pitfalls, but I am not perfect. I will not speak for Peff or any other reviewers whose jugement I trust, but I would be very surprised if any of them claimed he is perfectly even.

"Double" may only be showing that we do not have enough trusted maintainers; ideally I would like it to have "Triple" or more.

[Footnote]

*1* A few examples of core values.

 - we should make sure that future developers who wonder why a part
   of the code is how it is can find out what thought process
   brought the code into the current shape.

 - when we add something, we try not to overengineer or to shoot for
   unattainable perfection, but we still try to make sure we will
   not paint ourselves into an unescapable corner when we later want
   to extend it.