Re: [GSoC][PATCH v3 3/3] t0000: use test_line_count instead of wc -l
- From
Thomas Gummerer <t.gummerer@gmail.com>
- Date
- Mar 17, 2019, 20:06 UTC
- Message-ID
- <20190317200609.GA1216@hank.intra.tgummerer.com>
- In-Reply-To
- <87h8c1a1mi.fsf@evledraar.gmail.com>
On 03/17, Ævar Arnfjörð Bjarmason wrote:
Show 34 quoted lines
> > On Sun, Mar 17 2019, Jonathan Chang wrote: > > > Signed-off-by: Jonathan Chang <ttjtftx@gmail.com> > > --- > > 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.