Re: [PATCH v2] t9832,t2200: avoid using pipes in git commands

Eric Sunshine [off-list ref] 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.

;-)

quoted

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.

On Sun, Oct 18, 2020 at 9:04 PM Junio C Hamano [off-list ref] wrote:

Eric Sunshine [off-list ref] writes:

quoted

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.

;-)

quoted

quoted

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.

quoted

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.

Taken very seriously

Thanks.

-- 

Cheers!

Amanda  Shafack