Re: [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 23, 2023, 21:38 UTC
- Message-ID
- <xmqqsf89e8wz.fsf@gitster.g>
- In-Reply-To
- <ZOZnNDd2pMX6M2Au@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
Show 54 quoted lines
> 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.