Re: [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> This version is looking good. The main functionality is well-reasoned
> and straightforwardly implemented. One minor suggestion that you could
> consider squashing in is some test clean-up like so:
>
> --- 8< ---
> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
> index 7011e3a421..4dee71d6d5 100755
> --- a/t/t3501-revert-cherry-pick.sh
> +++ b/t/t3501-revert-cherry-pick.sh
> @@ -176,29 +176,27 @@ test_expect_success 'advice from failed revert' '
>  	test_cmp expected actual
>  '
>
> +test_expect_commit_msg () {
> +	echo "$@" >expect &&
> +	git log -1 --pretty=%s >actual &&
> +	test_cmp expect actual
> +}
> +
>  test_expect_success 'title of fresh reverts' '
>  	test_commit --no-tag A file1 &&
>  	test_commit --no-tag B file1 &&
>  	git revert --no-edit HEAD &&
> -	echo "Revert \"B\"" >expect &&
> -	git log -1 --pretty=%s >actual &&
> -	test_cmp expect actual &&
> +	test_expect_commit_msg "Revert \"B\"" &&
>  	git revert --no-edit HEAD &&
> -	echo "Reapply \"B\"" >expect &&
> -	git log -1 --pretty=%s >actual &&
> -	test_cmp expect actual &&
> +	test_expect_commit_msg "Reapply \"B\"" &&
>  	git revert --no-edit HEAD &&
> -	echo "Revert \"Reapply \"B\"\"" >expect &&
> -	git log -1 --pretty=%s >actual &&
> -	test_cmp expect actual
> +	test_expect_commit_msg "Revert \"Reapply \"B\"\""
>  '
>
>  test_expect_success 'title of legacy double revert' '
>  	test_commit --no-tag "Revert \"Revert \"B\"\"" file1 &&
>  	git revert --no-edit HEAD &&
> -	echo "Revert \"Revert \"Revert \"B\"\"\"" >expect &&
> -	git log -1 --pretty=%s >actual &&
> -	test_cmp expect actual
> +	test_expect_commit_msg "Revert \"Revert \"Revert \"B\"\"\""
>  '
>
>  test_expect_success 'identification of reverted commit (default)' '
> --- >8 ---
>
> To my eyes, it makes checking the subject of our revert commit against
> an expected value more readable by factoring out the echo, git log,
> test_cmp pattern.

Yeah it does make the test more concise and what is expected stand
out more clearly.  Good suggestion.