{"thread":{"id":"60224","subject":"[PATCH] t3404-rebase-interactive.sh: fix name of a rewording test","startedAt":"2023-09-11T21:39:27Z","lastAt":"2023-09-13T00:25:29Z","messageCount":7,"participants":["Oswald Buddenhagen","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"481707","messageId":"20230911122108.199207-1-oswald.buddenhagen@gmx.de","threadId":"60224","inReplyTo":null,"subject":"[PATCH] t3404-rebase-interactive.sh: fix name of a rewording test","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-09-11T12:21:08Z","receivedAt":"2023-09-11T21:39:27Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"The given test name made no sense to me at all; it seems to be a\nconcatenation of two unrelated things. This was introduced by\ncommit 0c164ae7a (\"rebase -i: add another reword test\", 20-08-20).\n\nSigned-off-by: Oswald Buddenhagen \n\n---\nactually, i don't understand what the test even does. shouldn't it, to\nmatch the description, actually dirty the tree and verify that the\noperation fails?\n\nCc: Phillip Wood \n---\n t/t3404-rebase-interactive.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 96a56aafbe..31ee5bc1f6 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -758,7 +758,7 @@ test_expect_success 'reword' '\n \tgit show HEAD~2 | grep \"C changed\"\n '\n \n-test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '\n+test_expect_success 'no uncommitted changes when rewording' '\n \tgit checkout E &&\n \ttest_when_finished \"git checkout @{-1}\" &&\n \t(\n-- \n2.42.0.419.g70bf8a5751\n\n"},{"id":"481731","messageId":"xmqqwmwws5gn.fsf@gitster.g","threadId":"60224","inReplyTo":"20230911122108.199207-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH] t3404-rebase-interactive.sh: fix name of a rewording test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-11T22:36:08Z","receivedAt":"2023-09-12T02:08:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen writes:\n\n> The given test name made no sense to me at all; it seems to be a\n> concatenation of two unrelated things. This was introduced by\n> commit 0c164ae7a (\"rebase -i: add another reword test\", 20-08-20).\n>\n> Signed-off-by: Oswald Buddenhagen \n>\n> ---\n> actually, i don't understand what the test even does. shouldn't it, to\n> match the description, actually dirty the tree and verify that the\n> operation fails?\n>\n> Cc: Phillip Wood \n\nAs a typofix the updated text looks more correct, but the above is a\nvery good point, so I'll let Phillip, whose 0c164ae7 (rebase -i: add\nanother reword test, 2021-08-20) introduced this test, react first.\n\nThanks for spotting and fixing.\n\n> ---\n> t/t3404-rebase-interactive.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 96a56aafbe..31ee5bc1f6 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -758,7 +758,7 @@ test_expect_success 'reword' '\n> \tgit show HEAD~2 | grep \"C changed\"\n> '\n> \n> -test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '\n> +test_expect_success 'no uncommitted changes when rewording' '\n> \tgit checkout E &&\n> \ttest_when_finished \"git checkout @{-1}\" &&\n> \t(\n"},{"id":"481752","messageId":"45ee4ad1-f7a3-43d8-99ef-329efc9fcdba@gmail.com","threadId":"60224","inReplyTo":"xmqqwmwws5gn.fsf@gitster.g","subject":"Re: [PATCH] t3404-rebase-interactive.sh: fix name of a rewording test","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-12T10:15:30Z","receivedAt":"2023-09-12T10:16:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 11/09/2023 23:36, Junio C Hamano wrote:\n> Oswald Buddenhagen writes:\n> \n>> The given test name made no sense to me at all; it seems to be a\n>> concatenation of two unrelated things. This was introduced by\n>> commit 0c164ae7a (\"rebase -i: add another reword test\", 20-08-20).\n>>\n>> Signed-off-by: Oswald Buddenhagen \n>>\n>> ---\n>> actually, i don't understand what the test even does. shouldn't it, to\n>> match the description, actually dirty the tree and verify that the\n>> operation fails?\n>>\n>> Cc: Phillip Wood \n> \n> As a typofix the updated text looks more correct, but the above is a > very good point, so I'll let Phillip, whose 0c164ae7 (rebase -i: add\n> another reword test, 2021-08-20) introduced this test, react first.\n\nThanks. There is a typo, but that typo is a missing \"and\", the title \nshould be\n\nno uncommitted changes when rewording and the todo list is reloaded\n\nThe purpose of the test is to ensure that\n\n (i) There are no uncommited changes when the editor runs. i.e. we\n commit without running the editor and then reword by amending\n that commit. This ensures that we have the same user experience\n whether or not the commit was fast-forwarded [1].\n\n(ii) That the todo list is re-read after the commit has been\n reworded. This is to allow the user to update the todo list while\n the rebase is paused for editing the commit message.\n\nAll of the action happens in the functions called from this test. \nset_reword_editor() sets up an editor that runs \"git diff --exit-code \nHEAD\" to ensure there are no uncommitted changes when it is run and also \nupdates the todo list. Then check_reworded_commits() checks that the \ncommits were reworded and that the todo list was updated.\n\nBest Wishes\n\nPhillip\n\n[1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/\n\n> Thanks for spotting and fixing.\n> \n>> ---\n>> t/t3404-rebase-interactive.sh | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n>> index 96a56aafbe..31ee5bc1f6 100755\n>> --- a/t/t3404-rebase-interactive.sh\n>> +++ b/t/t3404-rebase-interactive.sh\n>> @@ -758,7 +758,7 @@ test_expect_success 'reword' '\n>> \tgit show HEAD~2 | grep \"C changed\"\n>> '\n>> \n>> -test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '\n>> +test_expect_success 'no uncommitted changes when rewording' '\n>> \tgit checkout E &&\n>> \ttest_when_finished \"git checkout @{-1}\" &&\n>> \t(\n\n"},{"id":"481754","messageId":"20230912104237.271616-1-oswald.buddenhagen@gmx.de","threadId":"60224","inReplyTo":"45ee4ad1-f7a3-43d8-99ef-329efc9fcdba@gmail.com","subject":"[PATCH v2] t3404-rebase-interactive.sh: fix typos in title of a rewording test","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-09-12T10:42:36Z","receivedAt":"2023-09-12T10:42:45Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"This test was introduced by commit 0c164ae7a (\"rebase -i: add another\nreword test\", 2021-08-20). I didn't quite get what it was meant to do,\nso here's an explanation from Phillip:\n\nThe purpose of the test is to ensure that\n\n (i) There are no uncommited changes when the editor runs. I.e., we\n commit without running the editor and then reword by amending\n that commit. This ensures that we have the same user experience\n whether or not the commit was fast-forwarded [1].\n\n (ii) That the todo list is re-read after the commit has been reworded.\n This is to allow the user to update the todo list while the rebase\n is paused for editing the commit message.\n\n[1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/\n\nSigned-off-by: Oswald Buddenhagen \n\n---\nv2:\n- actually, it's kinda a new patch now\n\nCc: Phillip Wood \nCc: Junio C Hamano \n---\n t/t3404-rebase-interactive.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 96a56aafbe..bf94eb28b9 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -758,7 +758,7 @@ test_expect_success 'reword' '\n \tgit show HEAD~2 | grep \"C changed\"\n '\n \n-test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '\n+test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n \tgit checkout E &&\n \ttest_when_finished \"git checkout @{-1}\" &&\n \t(\n-- \n2.42.0.419.g70bf8a5751\n\n"},{"id":"481756","messageId":"ZQBIBVadrMfsO9v9@ugly","threadId":"60224","inReplyTo":"20230912104237.271616-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v2] t3404-rebase-interactive.sh: fix typos in title of a rewording test","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-09-12T11:14:13Z","receivedAt":"2023-09-12T11:15:35Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Tue, Sep 12, 2023 at 12:42:36PM +0200, Oswald Buddenhagen wrote:\n>so here's an explanation from Phillip:\n>\n>The purpose of the test is to ensure that\n>\n> (i) There are no uncommited changes when the editor runs. I.e., we\n\t\t\t ^\nyou may want to add the missing 't' here when applying.🙃\n\nregards\n"},{"id":"481761","messageId":"76c36744-b821-450b-9817-e0cd8811de65@gmail.com","threadId":"60224","inReplyTo":"20230912104237.271616-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v2] t3404-rebase-interactive.sh: fix typos in title of a rewording test","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-12T13:26:36Z","receivedAt":"2023-09-12T13:26:48Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 12/09/2023 11:42, Oswald Buddenhagen wrote:\n> This test was introduced by commit 0c164ae7a (\"rebase -i: add another\n> reword test\", 2021-08-20). I didn't quite get what it was meant to do,\n> so here's an explanation from Phillip:\n> \n> The purpose of the test is to ensure that\n> \n> (i) There are no uncommited changes when the editor runs. I.e., we\n> commit without running the editor and then reword by amending\n> that commit. This ensures that we have the same user experience\n> whether or not the commit was fast-forwarded [1].\n> \n> (ii) That the todo list is re-read after the commit has been reworded.\n> This is to allow the user to update the todo list while the rebase\n> is paused for editing the commit message.\n> \n> [1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/\n> \n> Signed-off-by: Oswald Buddenhagen \n\nThis looks good to me, Thanks\n\nPhillip\n\n> ---\n> v2:\n> - actually, it's kinda a new patch now\n> \n> Cc: Phillip Wood \n> Cc: Junio C Hamano \n> ---\n> t/t3404-rebase-interactive.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 96a56aafbe..bf94eb28b9 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -758,7 +758,7 @@ test_expect_success 'reword' '\n> \tgit show HEAD~2 | grep \"C changed\"\n> '\n> \n> -test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '\n> +test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n> \tgit checkout E &&\n> \ttest_when_finished \"git checkout @{-1}\" &&\n> \t(\n\n"},{"id":"481795","messageId":"xmqq8r9aly15.fsf@gitster.g","threadId":"60224","inReplyTo":"76c36744-b821-450b-9817-e0cd8811de65@gmail.com","subject":"Re: [PATCH v2] t3404-rebase-interactive.sh: fix typos in title of a rewording test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-13T00:25:26Z","receivedAt":"2023-09-13T00:25:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 12/09/2023 11:42, Oswald Buddenhagen wrote:\n>> This test was introduced by commit 0c164ae7a (\"rebase -i: add another\n>> reword test\", 2021-08-20). I didn't quite get what it was meant to do,\n>> so here's an explanation from Phillip:\n>> The purpose of the test is to ensure that\n>> (i) There are no uncommited changes when the editor runs. I.e.,\n>> we\n>> commit without running the editor and then reword by amending\n>> that commit. This ensures that we have the same user experience\n>> whether or not the commit was fast-forwarded [1].\n>> (ii) That the todo list is re-read after the commit has been\n>> reworded.\n>> This is to allow the user to update the todo list while the rebase\n>> is paused for editing the commit message.\n>> [1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/\n>> Signed-off-by: Oswald Buddenhagen \n>\n> This looks good to me, Thanks\n>\n> Phillip\n\nThanks, both. Will queue.\n"}]}