Re: [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts
From: Taylor Blau <hidden>
Date: 2023-08-23 20:09:21
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Mon, Aug 21, 2023 at 07:07:19PM +0200, Oswald Buddenhagen wrote:
To achieve these goals, the mechanism does not need to be particularly sophisticated. Therefore, more complicated alternatives which would "compress more efficiently" have not been implemented.
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. Thanks, Taylor