On Sun, Oct 18, 2020 at 10:42 AM Amanda Shafack via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> t9832,t2200: avoid using pipes in git commands
The subject is a bit confusing since pipes aren't used in Git
commands; instead, Git commands may be components of pipes. However,
even that is too imprecise. The issue this patch is addressing is that
we want to avoid Git commands _upstream_ in a pipe. It's perfectly
acceptable for the Git command to be the final element of a pipe since
the pipe returns the exit code of the final command. So, to be more
precise, the subject could say:
t2200,t9832: avoid using `git` upstream in a pipe
Nit: It's subjective, but it feels a bit more natural to list the test
numbers in ascending order rather than descending order, which is why
I swapped them around in the example above.
> When a git command is upstream in a pipe, an unexpected failure of
> the git command will go unnoticed.
>
> Write out the output of the git command to a file, so as to actively
> catch a failure of the git command.
It's easy to see from the patch itself that the output of the Git
command is now written to a file, so it's not necessary to say so in
the commit message. Therefore, the entire body of the commit message
could be written more succinctly, perhaps like this:
Avoid placing `git` upstream in a pipe since doing so throws away
its exit code, thus an unexpected failure may go unnoticed.
The actual patch itself looks fine, and these comments about the
commit message are quite minor, thus there probably is no need to
re-roll (though feel free to do so if you think the bit of extra
polishing of the commit message is worthwhile).
Thanks.
