Re: [PATCH 00/19] tests: fix broken &&-chains & abort loops on error

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 9, 2021, 19:17 UTC
Message-ID: <CAPig+cRD_DzisMo-8ZuT4NzESEe4i2vPk_1Y-_JTeV9rbdwkLg@mail.gmail.com>
In-Reply-To: <CABPp-BFM5ZbFAzVfvDE3=zm6Q4LN2fWthPP8WH5kbgVPSxomtA@mail.gmail.com>

On Thu, Dec 9, 2021 at 12:03 PM Elijah Newren <newren@gmail.com> wrote:

Show 13 quoted lines

> On Wed, Dec 8, 2021 at 11:39 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > Although the new chainlint implementation has been complete for several
> > months, I'm still working out how to organize its patch series. In the
> > meantime, _this_ patch series fixes problems discovered by the new
> > linter due to its improved coverage and extra semantic knowledge about
> > Git tests. As much as possible, I resisted the temptation to make
> > ancillary cleanups (including indentation fixes) to tests even when such
> > cleanups would be obvious improvements. Avoiding such unrelated cleanups
> > should make the long patches in this series, which touch a lot of tests,
> > easier to review (--color-words helps a lot here).
>
> I have to admit to starting to skim once I got to the last four
> patches, since they were a bit longer and all the same type of change.

Understandable. Those bulk-change patches tend to be mind-numbing to review, though `git diff --color-words` helps out somewhat (at least by making it easier to skim the changes).

Show 7 quoted lines

> You did an excellent job of explaining the changes and presented them
> in a logical fashion.  The few things I thought I caught, you've
> already answered were already correct.  I do think making the second
> commit message be a bit clearer about the importance of the ordering
> would be helpful.  Anyway:
>
> Reviewed-by: Elijah Newren <newren@gmail.com>

Thanks. I'll wait a couple days and resend with a clarified commit message for the second patch unless, perhaps, Junio would accept a resend of just that patch so I don't have to spam the list again.