Hi Denton, Thanks for the review. The patch was supposed to be in response to https://public-inbox.org/git/20190613151756.GA31952@xxxxxxxxxx/ but apparently I didn't use --in-reply-to correctly. I'll resubmit with the requested changes. -Michael On Sun, 16 Jun 2019 at 20:02, Denton Liu <liu.denton@xxxxxxxxx> wrote: > > Thanks for the patch, Michael! > > On Sat, Jun 15, 2019 at 07:40:39PM +0100, michael@xxxxxxxxx wrote: > > Subject: [PATCH] Use an intermediate file between between git blame and sed to avoid git blame's exit code being hidden. > > For your commit message, the usual convention is to first specify the > area you're working on followed by a colon and a brief summary. > Typically, the subject starts with a lowercase character and also > doesn't end with any punctuation. See [[describe-changes]] under > Documentation/SubmittingPatches for more details. > > 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. > > In addition, your commit message is missing a sign-off line. You can add > one by passing `-s` to git commit but you should read about what it > means in [[sign-off]] in SubmittingPatches. > > > From: Michael Platings <michael@xxxxxxxxx> > > > > --- > > t/t8014-blame-ignore-fuzzy.sh | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/t/t8014-blame-ignore-fuzzy.sh b/t/t8014-blame-ignore-fuzzy.sh > > index 1ff59624e9..13f3313710 100755 > > --- a/t/t8014-blame-ignore-fuzzy.sh > > +++ b/t/t8014-blame-ignore-fuzzy.sh > > @@ -332,7 +332,9 @@ test_expect_success setup ' > > for i in $(test_seq 2 $last_test); do > > eval title="\$title$i" > > test_expect_success "$title" \ > > - "git blame -M9 --ignore-rev $IGNOREME $i | sed -e \"$pick_author\" >actual && test_cmp expected$i actual" > > + "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. > > Thanks, > > Denton > > > + test_cmp expected$i actual" > > done > > > > # This invoked a null pointer dereference when the chunk callback was called > > @@ -357,7 +359,8 @@ test_expect_success 'Diff chunks with no suspects' ' > > > > test_write_lines 1 1 >expected && > > > > - git blame --ignore-rev $REV_2 --ignore-rev $REV_3 file | sed -e "$pick_author" >actual && > > + git blame --ignore-rev $REV_2 --ignore-rev $REV_3 file >output && > > + sed -e "$pick_author" <output >actual && > > > > test_cmp expected actual > > ' > > @@ -387,7 +390,8 @@ test_expect_success 'position matching' ' > > > > test_write_lines 1 1 2 2 >expected && > > > > - git blame --ignore-rev $REV_3 --ignore-rev $REV_4 file2 | sed -e "$pick_author" >actual && > > + git blame --ignore-rev $REV_3 --ignore-rev $REV_4 file2 >output && > > + sed -e "$pick_author" <output >actual && > > > > test_cmp expected actual > > ' > > @@ -424,7 +428,8 @@ test_expect_success 'preserve order' ' > > > > test_write_lines 1 2 3 >expected && > > > > - git blame --ignore-rev $REV_4 --ignore-rev $REV_5 file3 | sed -e "$pick_author" >actual && > > + git blame --ignore-rev $REV_4 --ignore-rev $REV_5 file3 >output && > > + sed -e "$pick_author" <output >actual && > > > > test_cmp expected actual > > ' > > -- > > 2.21.0 > >
