Re: [PATCH 00/29] t: detect and fix broken &&-chains in subshells

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 26, 2018, 22:31 UTC
Message-ID: <xmqq6025i3io.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAPig+cSW6jP3FtYpwf5bB4SM=qw6A3K9H3JPranJ_KMqH-AwZw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 14 quoted lines

> On Tue, Jun 26, 2018 at 3:38 PM Junio C Hamano <gitster@pobox.com> wrote:
>> I first looked at 29/29 and got heavily inclined to reject that
>> step, and then continued reading from 1/29 to around 15/29.
>>
>> I like these earlier changes that fix existing breakage, of course.
>> I also like many of the changes that simplify and/or modernise the
>> test scripts very much, but they are unusable as-is as long as their
>> justification is "chain-lint will start barfing on these constructs".
>
> Sorry, I'm having difficulty understanding.
>
> Are you saying that you don't want patches which exist merely to
> pacify --chain-lint? (For instance, 2/29 "t0001: use "{...}" block
> around "||" expression rather than subshell".)

Yes.

> Or are you saying that you don't like how the commit messages are
> worded, and that they should instead emphasize that the change is good
> for its own sake, without mentioning --chain-lint?

Yes, too.

For example, 03/29 is a good clean-up, and its value is not diminished even if we reject the subprocess munging --chain-lint in 29/29.

As opposed to 02/29 which mostly is about appeasing the "shell parser" in 29/29 (or you could justify it saying "one less fork and process" if that gives us a measurable benefit).