Re: [PATCH 2/4] t7004-tag.sh: re-arrange git tag comment for clarity

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 14, 2010, 17:32 UTC
Message-ID: <20101114173241.GB26459@burratino>
In-Reply-To: <1289745857-16704-3-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason wrote:

>                           I used ! instead of test_must_fail so that
> the GIT_EDITOR variable was only used in this command invocation,

	(
		GIT_EDITOR=cat &&
		export GIT_EDITOR &&
		test_must_fail ...
	)

>                                                                   and
> because the surrounding tests use this style.

Oh, okay. I had the test_must_fail version sitting in my local tree as part of the unsent part of the &&-chaining series http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160419

I am happy to see this fixed sooner.

[...]

Show 19 quoted lines

> +test_expect_success \
> +	'message in editor has initial comment: first line' '
>  	# check the first line --- should be empty
> -	first=$(sed -e 1q <actual) &&
> -	test -z "$first" &&
> +	echo >first.expect &&
> +	sed -e 1q <actual >first.actual &&
> +	test_cmp first.expect first.actual
> +'
> +
> +test_expect_success \
> +	'message in editor has initial comment: remainder' '
>  	# remove commented lines from the remainder -- should be empty
> -	rest=$(sed -e 1d -e '/^#/d' <actual) &&
> -	test -z "$rest"
> +	>rest.expect
> +	sed -e 1d -e '/^#/d' <actual >rest.actual &&
> +	test_cmp rest.expect rest.actual
>  '

Nice. Note that you've changed the semantics of the test here (probably for the better): the $() backquoting operator strips trailing newlines, while your sed expression does not.

I assume you've run the tests and made sure they still pass.

For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>