On 03/17, Ævar Arnfjörð Bjarmason wrote:
>
> On Sun, Mar 17 2019, Jonathan Chang wrote:
>
> > Signed-off-by: Jonathan Chang <ttjtftx@xxxxxxxxx>
> > ---
> > t/t0000-basic.sh | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> > index 47666b013e..3de13daabe 100755
> > --- a/t/t0000-basic.sh
> > +++ b/t/t0000-basic.sh
> > @@ -1136,8 +1136,8 @@ test_expect_success 'git commit-tree omits duplicated parent in a commit' '
> > parent=$(sed -n -e "s/^parent //p" -e "/^author /q" actual | sort -u) &&
> > test "z$commit0" = "z$parent" &&
> > git show --pretty=raw $commit2 >actual &&
> > - numparent=$(sed -n -e "s/^parent //p" -e "/^author /q" actual | wc -l) &&
> > - test $numparent = 1
> > + sed -n -e "s/^parent //p" -e "/^author /q" actual >parents &&
> > + test_line_count = 1 parents
> > '
> >
> > test_expect_success 'update-index D/F conflict' '
> > @@ -1146,8 +1146,7 @@ test_expect_success 'update-index D/F conflict' '
> > mv tmp path2 &&
> > git update-index --add --replace path2 path0/file2 &&
> > git ls-files path0 >actual &&
> > - numpath0=$(wc -l <actual) &&
> > - test $numpath0 = 1
> > + test_line_count = 1 actual
> > '
>
> ...of course reading this series in sequence I find that 50% of my
> suggestions for 2/3 are then done in this patch... :)
Indeed. I think doing this in a separate patch is a good idea, as it
makes the previous patch slightly easier to review imho. But I think
something to take away from this for Jonathan would be that this
should have been described in the commit message of the previous
commit. Maybe something like
This commit doesn't make any additional simplifications, such as
using the test_line_count function for counting the lines in the
output. These simplifications will be made in a subsequent commit.
in addition to the existing commit message would have helped save a
bit of review effort.
