Re: [PATCH v2 07/10] t4205: cover `git log --reflog -z` blindspot
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Nov 8, 2019, 21:58 UTC
- Message-ID
- <CAPig+cQ3FrxUGiFXooaiZ+po0qK6tBOda5apCs4=H0mdWxAKOg@mail.gmail.com>
- In-Reply-To
- <20191108214711.GA27310@generichostname>
On Fri, Nov 8, 2019 at 4:47 PM Denton Liu <liu.denton@gmail.com> wrote:
Show 10 quoted lines
> 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.
Show 13 quoted lines
> > > + 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.