Re: [PATCH] t4205: don't exit test script on failure

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 3, 2022, 00:46 UTC
Message-ID: <xmqq5yetuxek.fsf@gitster.g>
In-Reply-To: <xmqq5yeur32x.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 13 quoted lines

>> In my mind that's better than a "LGTM" or "Reviewed-by". Those are both
>> versions of "I looked over your work", but if you independently come up
>> with the same thing that's usually a stronger sign that the proposed
>> solution is a good one.
>
> Not necessarily.
>
> Past effort that did not fare well needs to be re-examined to make
> sure it was not picked up because it was crappy, because two people
> independently coming up with the same crappiness does not help us
> build more confidence.  Instead of forcing other reviewers waste
> their time looking at older threads, it would help to explain what
> you find good in the patch you are reviewing.

Related to this, another thing you often do is very helpful: to say that the patch being proposed solves the same problem another patch that is already in our tree solved in a different part of the code base. If it was good for another part of the system, it is likely that the same solution may be a good fit for the part being touched as well.

Compared to that, referring to an earlier patch that failed to hit our code base is not all that helpful.

Thanks.