Re: [GSoC][PATCH v4 4/5] t0000: use test_cmp instead of "test" builtin
- Date: Sun, 31 Mar 2019 19:57:24 +0100
- From: Thomas Gummerer <t.gummerer@xxxxxxxxx>
- Subject: Re: [GSoC][PATCH v4 4/5] t0000: use test_cmp instead of "test" builtin
On 03/31, jonathan chang wrote:
> 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.
This is what running tests, and reviewing your own code are good for :)
> 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.
Right, appending patches using 4/3 for example can be done in some
rare cases, but since you already modified patch 2/2, a new new
iteration is required anyway.
So yeah improving the overall series is always a good thing.
Especially since I see you already included the range-diff, it's not
very hard for reviewers to see what changed in the last iteration.