Re: [PATCH 00/15] Use test_config

From: Yann Droneaud <ydroneaud@opteya.com>
Date: Mar 27, 2013, 16:19 UTC
Message-ID: <ec6c6b2101cf035b5dea7f7110ddf706@meuh.org>
In-Reply-To: <7vmwtoq3xb.fsf@alter.siamese.dyndns.org>

Hi,

Le 27.03.2013 16:05, Junio C Hamano a écrit :

Show 7 quoted lines

> Yann Droneaud <ydroneaud@opteya.com> writes:
>
>> Tested against master, 7b592fadf1e23b10b913e0771b9f711770597266
>
> Is this because I suggested you to clean things up while you were
> touching in a vicinity of something that could use this clean-up?
>

Yes, grep'ing shows others usage of the test_config pattern. I patched them all.

Show 6 quoted lines

> If so, please first clean _that_ script in a patch, and then add the
> change you wanted to do in another patch, as a single two-patch
> series, without touching anything else that is not related to that
> change.  The patch to t7600 is the one that needs to become two
> patches, one to clean up and the other to add tests for --no-ff.
>

Actually the initial patch adding test for --no-ff-only is not part of this series.

Patch against t7600 has a special note about a strange behavor found while testing test_config "anyware", that's why there's somes line added to the test and a note in the commit message.

I was waiting for your opinion on this change in the test, but more, on the difference of behavior exhibited in the patched test "merge log message":

   git merge --no-log
   git show -s --pretty=format:%b HEAD

vs

   git merge --no-ff --no-log
   git show -s --pretty=format:%b HEAD

First produce an empty file, while the second produce an empty line.

This was revealed by changing test "merge c0 with c1 (ff overrides 
no-ff)
-    git config branch.master.mergeoptions "--no-ff" &&
-    test_config branch.master.mergeoptions "--no-ff" &&

I could split this patch in a first patch that add the behavor test to "merge log message" test, than I could rebase the patch series against. And later, submit my proposition for new tests in t7600 regarding --no-ff-only and tags.

Show 7 quoted lines

> The rest, as a separate "only cleaning up, doing nothing else"
> series, are fine as a follow-up, but please make sure that they do
> not touch anything in-flight (one easy way to check is to see "git
> diff --name-only maint pu -- t/").  I would prefer to see "clean-up
> only" changes that introduce unnecessary conflicts with other real
> features and fixes held off until the dust settles.
>

It's a good advice that fit perfectly in Documentation/SubmittingPatches.

Regards.

-- 
Yann Droneaud
OPTEYA