Re: [PATCH v2 07/10] t4205: cover `git log --reflog -z` blindspot

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Nov 8, 2019 at 4:47 PM Denton Liu <liu.denton@gmail.com> wrote:
> On Fri, Nov 08, 2019 at 03:36:25PM -0500, Eric Sunshine wrote:
> > For completeness, the above example also drops the unnecessary 'revs'
> > variable, uses double quotes rather than single when interpolating $p,
> > and makes the loop early-exit a bit more idiomatic.
>
> The reason why I pulled out `revs` was because putting the rev-list
> within the for-loop would cause its exit code to be lost if it ever
> failed. Since I've been cleaning up test scripts to not lose exit codes
> from git commands, I figured it'd be a bad idea to introduce an instance
> here ;)

Make sense. Ignore that suggestion of mine.

> > > +       git log -z --pretty=oneline --reflog >actual &&
> > > +       # no trailing NUL
> >
> > To what is this comment referring?
>
> I wanted to point out how in the oneline case, the trailing NUL is
> already included, unlike in the multiple cases, so we don't need to
> output another one.
>
> I'll rewrite this as
>
>         # the trailing NUL is already produced so we don't need to
>         # output another one

That will be clearer. It might be even clearer if that comment comes
before the "git log -z" invocation rather than after (though that's
subjective). Thanks.