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

From: Denton Liu <liu.denton@gmail.com>
Date: Nov 8, 2019, 21:47 UTC
Message-ID: <20191108214711.GA27310@generichostname>
In-Reply-To: <CAPig+cQDMSwP5-D-=LgjBPH2kJK16Fv4c619Pg6OTU95CQ9sgw@mail.gmail.com>

Hi Eric,

On Fri, Nov 08, 2019 at 03:36:25PM -0500, Eric Sunshine wrote:

[...]

Show 9 quoted lines

>     for r in $(git rev-list --reflog)
>     do
>         git show -s --pretty="$p" "$r" &&
>         printf "\0" || return 1
>     done >expect &&
> 
> 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 ;)

Thanks for the other two suggestions, though. I didn't know that you could write it like that.

> > +       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

Thanks for your feedback,

Denton