Re: [PATCH 1/2] t3404: extra checks and s/! git/test_must_fail git/

From: しらいしななこ <nanako3@lavabit.com>
Date: Jun 20, 2008, 22:18 UTC
Message-ID: <20080621071812.6117@nanako3.lavabit.com>
In-Reply-To: <20080620190037.GE7369@leksak.fem-net>

Quoting Stephan Beyer <s-beyer@gmx.net>:

Show 15 quoted lines

> Hi,
>
>> > @@ -380,7 +397,7 @@ test_expect_success 'interrupted squash works as expected' '
>> >  	! FAKE_LINES="1 squash 3 2" git rebase -i HEAD~3 &&
>> 
>> These can be converted to use test_must_fail by using a sub-shell
>> as Junio demonstrated:
>> 
>> 	(
>> 		FAKE_LINES="1 squash 3 2" &&
>> 		export FAKE_LINES &&
>> 		test_must_fail git rebase -i HEAD~3
>> 	) &&
>
> Perhaps I'm not consequent, but I thought that it's not worth it ;-)

Doesn't that logic make the other s/!/test_must_fail/ changes also not worth it? What is the reason behind the change?

I think your subject line and the message is worse than your previous one. You are saying *HOW* you changed it, without saying *WHY* nor *WHAT FOR*.

I may have written your log message like this:

	Subject: t3404: tighten git-rebase tests

	In preparation for rewriting git-rebase in C, replace the way a
	failure is currently detected with "! git" to use test_must_fail
	so that we do not confuse a broken rebase that dumps core with a
	correctly failing one.

although I do not know if you are rewriting git-rebase in C (^_^). The point I learned from this project is to say why it is done that way, not how you did it. The latter can be seen in the diff.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/