Subject: Re: [PATCH v2 00/33] Yet more preparation for reference backends

From: Michael Haggerty <[email-masked]>
Date: Mon, 9 May 2016 23:50:35 +0200

On 05/09/2016 11:05 PM, Junio C Hamano wrote:
> David Turner <[email-masked]> writes:
>> [...]
>> I generally like to put the bug fixes before the tests for those fixes
>> (so that bisect on the complete suite works).  But maybe the git policy
>> is different.
> 
> The Git policy only asks not to break bisection.
> 
> As long as patch that adds a new test that comes before a patch that
> fixes the issue marks the new test with test_expect_failure, and a
> later patch that fixes the issue turns it into test_expect_success,
> bisection would not break.
> 
> The "demonstrate an existing breakage first" order makes it slightly
> easier to review and follow a long series, as it forces the reviewer
> to see the issue first and think about possible avenues to solve it
> for themselves, before seeing a paticular solution.  For a trivial
> single-issue fix, it is not necessary (including a fix and a test to
> protect the fix from future breakage in the same patch is a norm).

I find it useful to add the broken test in a separate patch, because it
is then easy to cherry pick that patch to other versions of Git to
discover which ones are also affected by the problem. If the addition of
the test is combined with the fix, then the patch would more often fail
to apply to other versions due to conflicts at the locations of the fix,
and even if it applied, you wouldn't learn whether that version of git
was broken but the breakage was fixed by the same fix, or whether it
wasn't broken in the first place and the fix was unnecessary.

Michael