Re: [RFC/ PATCH 4/5] t3030: update porcelain expected message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Diane Gasselin <diane.gasselin@xxxxxxxxxxxxxxx> writes:

> From: Diane <diane.gasselin@xxxxxxxxxxxxxxx>

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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]