Re: [PATCH 00/15] generalize chainlint self-tests

From: Elijah Newren <newren@gmail.com>
Date: Dec 15, 2021, 00:00 UTC
Message-ID: <CABPp-BFNEPkEaY-MT1Ot7EzHkW=7FHGbJxU=pr226M43BvHwTw@mail.gmail.com>
In-Reply-To: <20211213063059.19424-1-sunshine@sunshineco.com>

On Sun, Dec 12, 2021 at 10:31 PM Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 14 quoted lines

>
> A while back, Peff successfully nerd-sniped[1] me into tackling a
> long-brewing idea I had about (possibly) improving `chainlint`
> performance by linting all tests in all scripts with a single command
> invocation instead of running `sed` 25300+ times (once for each test).
> A description of some of the important features of the new linter can be
> found at [2].
>
> 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 makes it possible for the new linter to
> re-use the existing chainlint self-tests. It does so by cleansing the
> self-test ".test" and ".expect" files of implementation details and
> limitations specific to chainlint.sed.

I read through the patches in this series, and didn't note any problems. However, my knowledge of sed is just the basics. Even in a few cases where regexes were all that were really involved, the regexes were lengthy enough that my eyes just glazed over. So, my review is kind of superficial, but the preparatory patches certainly seem good to me, and the commit messages are well explained, and the non-sed changes are consistent with the described changes, and the easier sed stuff looked good. It's clear you put a lot of care into carefully explaining and dividing up the patches in a nice and logical manner.