Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 7, 2020, 22:20 UTC
Message-ID: <CAPig+cTm8foBehQC3wVDN6Lx1SO8EGhApVbUfQ0iaBnDU7OxtA@mail.gmail.com>
In-Reply-To: <xmqq364yf6tt.fsf@gitster.c.googlers.com>

On Fri, Aug 7, 2020 at 6:07 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
> > Consequently, I'm not in a
> > position to say whether or not there are any fundamental issues with
> > the changes made by any of these patches.
>
> Sorry, that wasn't what I meant.  A reroll would be done with an
> updated test, so I was asking your eyeball once again on that.

I eyeballed the test in the re-rolled [2/5] from a purely style perspective (not a content perspective), and it looks fine. I could say (and could previously have said) something about the lost exit codes from the $(git rev-parse ...) invocations within the here-doc in order to save Denton the trouble of eradicating them, but that seems relatively minor. Re-thinking the unindented here-docs on the other tests, I wonder now if that they might be easier to read if indented with \-EOF (rather than using plain EOF), as well:

    test_output_expect_success '--first-parent' 'git rev-list ...' <<-\EOF
        E
        ...
        e8
        EOF

but I doubt that such a minor style change is worth a re-roll (and I wouldn't ask Aaron to re-roll just for that).