From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Wed, 09 Jun 2010 20:40:20 GMT
Subject: Re: [RFC/ PATCH 4/5] t3030: update porcelain expected message
Message-ID: <vpq1vcgym6j.fsf@bauges.imag.fr>
In-Reply-To: <1276087446-25112-6-git-send-email-diane.gasselin@ensimag.imag.fr>

Diane Gasselin <diane.gasselin@ensimag.imag.fr> writes:

> From: Diane <diane.gasselin@ensimag.imag.fr>

You did something strange with git format-patch or send-email. This
From header should appear in the header of your email, but not in the
body.

> As porcelain messages have been changed, the expected porcelain message
> tested in this test needs to be changed.

We usually try to have the test-suite pass for each commit (so that
"git bisect" can be used easily among other reasons). So, you probably
want to squash this patch with the one that actually changes the
message. Also, I think it eases reviewing: the changes to the
test-suite can be seen as a specification (particularly clear here: we
know what the rest of the patch serie does reading this patch).

Another trick is to set the tests as "test_expect_failure" before
introducing the feature, and mark them as "test_expect_success" when
appropriate. This way, you can add new tests before adding the
feature, without introducing broken commits.

> +cat> expected2 <<EOF
> +error: Your local changes to the files:
> +	a
> +would be overwritten by merge.
> +EOF

I'd have phrased it like this:

error: Your local changes to these files would be overwritten by merge:
	a

to avoid splitting the message in two parts. It's more consistant with
the rest of Git (git status or git reset for example). Also, your
version would become hard to read if the file list is long.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/