From: Jakub Narebski <jnareb@gmail.com>
Date: Sat, 25 Sep 2010 07:56:24 GMT
Subject: Re: [RFC PATCHv2 09/16] t4026 (color): add missing &&
Message-ID: <m3d3s2gsub.fsf@localhost.localdomain>
In-Reply-To: <20100925040227.GA21416@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Sep 24, 2010 at 10:55:08PM +0000, Ævar Arnfjörð Bjarmason wrote:
> 
> > On Fri, Sep 24, 2010 at 22:22, Elijah Newren <newren@gmail.com> wrote:
> > > FIXME: Is it intended that this git config --unset fail?  I had to add a
> > > test_might_fail as well.
> > 
> > Presumably this is to work around the user having diff.color in his
> > config or something? Should we be leaking like that?
> 
> No, it was to clean up from previous tests, which set diff.color.new, to
> make sure we had a clean environment.  So it should have been might_fail
> (but it predated might_fail, which is why it wasn't).

Shouldn't modern test script use 'test_when_finished <script>' for
cleanup instead?
 
> But look at 8b124135, which came later. It changed the color() and
> invalid_color() helpers to no longer use diff.color.new, so that cleanup
> is now obsolete. So that line can simply be removed now.

Well, there is that.

-- 
Jakub Narebski
Poland
ShadeHawk on #git