Re: [PATCH] log: if --decorate is not given, default to --decorate=auto

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 23, 2017, 15:54 UTC
Message-ID: <xmqqfui4f0pp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAMMLpeR-zCpL5Gx=BoK8G9_wL2TBe-wD3VnsAShAuVVzS=Nirg@mail.gmail.com>

Alex Henrie <alexhenrie24@gmail.com> writes:

Show 11 quoted lines

> 2017-03-22 10:54 GMT-06:00 Junio C Hamano <gitster@pobox.com>:
>> Alex Henrie <alexhenrie24@gmail.com> writes:
>>> No problem. Do I need to submit a second version of the patch with a
>>> test for `git -p log`?
>>
>> You do want to protect this "without an option, we default to
>> 'auto'" feature from future breakage, no?
>
> Yes, but I need to know whether you want a v2 of this patch with all
> of the changes including the new test, or a second patch that depends
> on the first patch and only adds the new test.

Sorry, I misunderstood the question.

In general, we prefer to have tests that protects the updated behaviour in the same patch that makes code changes that brings in the new behaviour, i.e. a single v2 patch with new test would be more appropriate in this case.

When people work on a large bugfix, especially one that needs multiple steps, we sometimes see a patch that adds new tests that describe the desired behaviour as failing tests first, and then subsequent patches to the code to update the behaviour flip "test_expect_failure" to "test_expect_success" as they fix the behaviour. But for a small change like this one, that approach is inappropriate.

When a patch that was reviewed, deemed good enough and has been already merged to the 'next' branch later turns out that it needs further work (like "we do need some tests"), we do such necessary updates as separate follow-up patches, simply because we promise that 'next' won't be rewound and are not allowed to replace patches. But this one is not yet in 'next', so we can freely take a replacement patch.

Hope this message makes it clear enough?

Thanks.