On Sun, Mar 31, 2019 at 3:38 AM Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote: > > On 03/30, Jonathan Chang wrote: > > Signed-off-by: Jonathan Chang <ttjtftx@xxxxxxxxx> > > --- > > t/t0000-basic.sh | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh > > index 3de13daabe..49923c5ff1 100755 > > --- a/t/t0000-basic.sh > > +++ b/t/t0000-basic.sh > > @@ -1118,16 +1118,18 @@ P=$(test_oid root) > > > > test_expect_success 'git commit-tree records the correct tree in a commit' ' > > commit0=$(echo NO | git commit-tree $P) && > > - git show --pretty=raw $commit0 >actual && > > This line has been introduced in a previous commit. If the file was > called 'output' there already, I think that patch would be just as > understandable, but this diff would be a little less noisy. Make sense. I tried to make patches from last iteration untouched, so I don't break anything. And I was wondering since I'm only appending patches, if I should also append the PATCH number as [PATCH v3 4/3], to reduce the number of emails. Now I realize that by making it a new iteration, we can also make some improvement to reduce the patch noise. > > > - tree=$(sed -n -e "s/^tree //p" -e "/^author /q" actual) && > > - test "z$tree" = "z$P" > > + git show --pretty=raw $commit0 >output && > > + echo "$P" >expect && > > + sed -n -e "s/^tree //p" -e "/^author /q" output >actual && > > I'd find it a bit more natural to either first create the expect file, > and then do the 'git show' and 'sed' invocations in two subsequent > lines, or do them first, and then create the expect files, rather than > interleaving them. > > I'm not sure either of these by itself is worth a new iteration, > unless there is also something else to fix up. But it's something > that you might want to keep in mind for future patches. I thought about all three options, starting with expect files being the last, but found that most test_cmp seem to have expect file created first, then decided to make the creation of actual and expect be on consecutive lines. Now I would probably move the creation expect to the last line. Thanks for the review and suggestions. > > > + test_cmp expect actual > > ' > > > > test_expect_success 'git commit-tree records the correct parent in a commit' ' > > commit1=$(echo NO | git commit-tree $P -p $commit0) && > > - git show --pretty=raw $commit1 >actual && > > - parent=$(sed -n -e "s/^parent //p" -e "/^author /q" actual) && > > - test "z$commit0" = "z$parent" > > + git show --pretty=raw $commit1 >output && > > + echo "$commit0" >expect && > > + sed -n -e "s/^parent //p" -e "/^author /q" output >actual && > > + test_cmp expect actual > > ' > > > > test_expect_success 'git commit-tree omits duplicated parent in a commit' ' > > -- > > 2.21.0 > >
