Denton Liu <liu.den...@gmail.com> writes:

> For yours, I would reword your commit message to something like
>
>       t8014: avoid git command in upstream pipe
>       
>       Use an intermediate file between between git blame and sed to avoid
>       git blame's exit code being hidden.

I agree that the main "points" of this patch that should be
highlighted on the title line are that it is about a test, and it is
about not hiding a failure of a Git command by placing it on the
upstream side of a pipe---the above title is very nicely written.

>> +    "git blame -M9 --ignore-rev $IGNOREME $i >output &&
>> +    sed -e \"$pick_author\" <output >actual &&
>
> We should take advantage of the fact that sed can open its own input
> here. So we should drop the `<` and just pass the filename to sed. Same
> applies to the below.

While I do not think it matters too much in this case, I agree it is
a good habit to get into, because it would give the command a chance
to produce a better error diagnosis (i.e. "malformed input on line
X" vs "malformed input on line X in file F"), when it wants to
report an error in input, if we give the name of the file to open to
the command instead of an already-open file descriptor, for one thing.


Reply via email to