Re: [PATCH 0/2] Simple fixes to t7406

From: Elijah Newren <newren@gmail.com>
Date: Aug 7, 2018, 13:40 UTC
Message-ID: <CABPp-BGQowPCMA2msTQ8pSQgak+7gig7qeq0dN-ZGU7K+ceCyw@mail.gmail.com>
In-Reply-To: <CAN0heSqNFJm7cJ8SpTXLVi_0CFR5fphAT0D34a1k5i8d5T9E_g@mail.gmail.com>

On Tue, Aug 7, 2018 at 12:50 AM, Martin Ågren <martin.agren@gmail.com> wrote:

Show 13 quoted lines

> On 6 August 2018 at 17:25, Elijah Newren <newren@gmail.com> wrote:
>> Changes since v1:
>>   - Simplify multiple tests using diff --name-only instead of diff --raw;
>>     these tests are only interested in which file was modified anyway.
>>     (Suggested by Junio)
>>   - Avoid putting git commands upstream of a pipe, since we don't run under
>>     set -o pipefail (Suggested by Eric)
>>   - Avoid using test_must_fail for system binaries (Pointed out by
>>     both Eric and Junio)
>>
>> Elijah Newren (2):
>
> I'm not sure what's up with the count of 2 vs 3.

I started writing a cover letter, then realized I missed something in the original review. Fixed it up by adding another patch, but decided to just manually "fix up" my cover letter to match -- and missed something. Oops.

Show 8 quoted lines

>>   t7406: simplify by using diff --name-only instead of diff --raw
>>   t7406: avoid having git commands upstream of a pipe
>>   t7406: make a test_must_fail call fail for the right reason
>
> The subject of the final patch doesn't quite match its content anymore.
> The problematic test_must_fail is dropped, so it can no longer fail.
> Maybe something like "t7406: fix call that was failing for the wrong
> reason", or just s/test_must_fail// in what you have.

Good point; I'll fix it up.