Re: [PATCH v2] t9832,t2200: avoid using pipes in git commands
On Sun, Oct 18, 2020 at 9:04 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 39 quoted lines
>
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> > 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.
>
> Yup.
>
> > 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).
>
> IIUC, the microproject experience aims new contributors to get used
> to the style of communication that happens during review cycles of a
> typical topic, using a trivial dip-the-toes-in-the-water problem as
> an example. I'd rather not to see contributors get into the habit
> of leaving loose ends and have somebody else clean after them.
> Thanks.
--
Cheers!
Amanda Shafack