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
