Re: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState

From: Martin Erik Werner <martinerikwerner@gmail.com>
Date: Feb 13, 2013, 20:40 UTC
Message-ID: <1360788036.22870.4.camel@mas>
In-Reply-To: <7vmwv86le8.fsf@alter.siamese.dyndns.org>

On Wed, 2013-02-13 at 11:53 -0800, Junio C Hamano wrote:

Show 22 quoted lines

> Martin Erik Werner <martinerikwerner@gmail.com> writes:
> 
> >> Strictly speaking, you have 6 not 4 combinations (shell variable
> >> set/unset * config missing/set to false/set to true).  I think these
> >> additional tests cover should all 6 because "config missing" case
> >> should already have had tests before bash.showDirtyState was added.
> >> 
> >
> > Indeed, I only mentioned 4 since the other ones existed already, and I
> > didn't change them, but maybe it should be mentioned as "combined with
> > previous tests (...) cover all 6 combinations (...)" then?
> 
> It should be sufficient to change the third line of your original to
> say "the config option being missing/enabled/disabled, given a dirty
> file." and nothing else, I think.
> 
> >> Sign-off?
> >
> > Ah, just forgot the -s flag on that commit, yes it should be Signed-off
> > by me.
> 
> OK, I'll locally amend the patch.  Thanks.

Ok, so I shouldn't reroll them with s/unset -v/sane_unset/ and reworded commits + sign-off then, I can if you prefer that?

-- 
Martin Erik Werner <martinerikwerner@gmail.com>