Hi,
On Mon, Aug 13, 2018 at 1:28 PM SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote:
> On Tue, Aug 7, 2018 at 6:49 PM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> > Since folks like to notice other problems with t7406 while reading my
> > patches, here's a challenge:
> >
> > Find something *else* wrong with t7406 that neither I nor any of the
> > reviewers so far have caught that could be fixed.
>
> Well, I'd hate to be that guy... but since those who already
> commented on previous rounds are not explicitly excluded from the
> challenge, let's see.
>
> - There are still a few command substitutions running git commands,
> where the exit status of that command is ignored; just look for the
> '[^=]$(' pattern in the test script.
>
> (Is not noticing those cases considered as "flubbing"?)
Hmm, borderline.
> - The 'compare_head' helper function defined in this test script looks
> very similar to the generally available 'test_cmp_rev' function,
> which has the benefit to provide some visible output on failure
> (though, IMO, not a particularly useful output, because the diff of
> two OIDs is not very informative, but at least it's something as
> opposed to the silence of 'test $this = $that").
>
> Now, since 'compare_head' always compares the same two revisions,
> namely 'master' and HEAD, replacing 'compare_head' with an
> appropriate 'test_cmp_rev' call would result in repeating 'master'
> and 'HEAD' arguments all over the test script. I'm not sure whether
> that's good or bad. Anyway, I think that 'compare_head' could be
> turned into a wrapper around 'test_cmp_rev'.
Ooh, that does sound better.
> > - You get bonus points if that thing is in the context region for
> > one of my five patches.
> > - Extra bonus points if the thing needing fixing was on a line I
> > changed.
> > - You win outright if it's something big enough that I give up and
> > request to just have my series merged as-is and punt your
> > suggested fixes down the road to someone else.
>
> Well, there's always the indentation of the commands run in subshells,
> which doesn't conform to our coding style...
>
> Gah, now you made me that guy ;)
I read this on Monday and got a really good laugh. I meant to fix it
up, but fell asleep too soon the first couple nights...and now this
series is in next anyway and there are a couple other git things that
have my attention. You have pointed out a couple additional nice
fixups, like you always do, but I think at this point I'm just going
to declare you the winner and label these as #leftoverbits.
Thanks for always thoroughly looking over the testcase patches and
your constant work to improve the testsuite.
