Re: [GSoC][PATCH v4 4/5] t0000: use test_cmp instead of "test" builtin

From: Thomas Gummerer <t.gummerer@gmail.com>
Date: Mar 31, 2019, 18:57 UTC
Message-ID: <20190331185724.GW32487@hank.intra.tgummerer.com>
In-Reply-To: <CAOAu_YKMuy2vWQqZz+=-7JGhce9BEdMqsr8HOKZOJVq=chpqVw@mail.gmail.com>

On 03/31, jonathan chang wrote:

Show 24 quoted lines

> On Sun, Mar 31, 2019 at 3:38 AM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> >
> > On 03/30, Jonathan Chang wrote:
> > > Signed-off-by: Jonathan Chang <ttjtftx@gmail.com>
> > > ---
> > >  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 :)

Show 5 quoted lines

>                             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.