On Fri, Nov 8, 2019 at 4:47 PM Denton Liu <liu.denton@xxxxxxxxx> 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.