{"thread":{"id":"50404","subject":"[PATCH 0/1] rebase: drop the unwanted -y","startedAt":"2019-02-05T11:49:07Z","lastAt":"2019-02-06T19:28:58Z","messageCount":6,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"368522","messageId":"pull.118.git.gitgitgadget@gmail.com","threadId":"50404","inReplyTo":null,"subject":"[PATCH 0/1] rebase: drop the unwanted -y","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-02-05T11:49:03Z","receivedAt":"2019-02-05T11:49:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I totally missed that this patch, despite my pledge to not include it, made\nit into master. Sorry about that.\n\nJohannes Schindelin (1):\n Revert \"rebase: introduce a shortcut for --reschedule-failed-exec\"\n\n Documentation/git-rebase.txt | 6 ------\n builtin/rebase.c | 21 ---------------------\n git-legacy-rebase.sh | 6 ------\n t/t3418-rebase-continue.sh | 3 ---\n 4 files changed, 36 deletions(-)\n\n\nbase-commit: b5101f929789889c2e536d915698f58d5c5c6b7a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-118%2Fdscho%2Frevert-rebase-y-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-118/dscho/revert-rebase-y-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/118\n-- \ngitgitgadget\n"},{"id":"368523","messageId":"e61ebc30605e21ce71623903bc9c850fd964e826.1549367342.git.gitgitgadget@gmail.com","threadId":"50404","inReplyTo":"pull.118.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] Revert \"rebase: introduce a shortcut for --reschedule-failed-exec\"","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-02-05T11:49:04Z","receivedAt":"2019-02-05T11:49:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen contributing the patch series, the cover letter tried to convey\nclearly that the patch introducing the shortcut -y was included only to\nshow that it is possible, with a slight bias against it.\n\nDuring the review, there were a couple reviewers who agreed with this\nsentiment, and the author was happy that this patch was not needed and\nconcurred that it should be dropped. See e.g. Stefan Beller's reply:\n\n\nHowever, it slipped by the original patch author (yours truly) that the\npatch *was* included when the branch made it to `next` and then when it\nmade it to `master`.\n\nSo let's back out that patch before it even slips into an official\nrelease (in which case we would even have to support this unwanted\nflag).\n\nThis reverts commit 81ef8ee75d5f348d3c71ff633d13d302124e1a5e.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-rebase.txt | 6 ------\n builtin/rebase.c | 21 ---------------------\n git-legacy-rebase.sh | 6 ------\n t/t3418-rebase-continue.sh | 3 ---\n 4 files changed, 36 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 4dd5853d6e..44ffe2c8c5 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -462,12 +462,6 @@ without an explicit `--interactive`.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n--y ::\n-\tThis is the same as passing `--reschedule-failed-exec` before\n-\t`-x `, i.e. it appends the specified `exec` command and\n-\tturns on the mode where failed `exec` commands are automatically\n-\trescheduled.\n-\n --root::\n \tRebase all commits reachable from , instead of\n \tlimiting them with an . This allows you to rebase\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 774264bae8..7c77a80687 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -760,23 +760,6 @@ static int parse_opt_interactive(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-struct opt_y {\n-\tstruct string_list *list;\n-\tstruct rebase_options *options;\n-};\n-\n-static int parse_opt_y(const struct option *opt, const char *arg, int unset)\n-{\n-\tstruct opt_y *o = opt->value;\n-\n-\tif (unset || !arg)\n-\t\treturn -1;\n-\n-\to->options->reschedule_failed_exec = 1;\n-\tstring_list_append(o->list, arg);\n-\treturn 0;\n-}\n-\n static void NORETURN error_on_missing_default_upstream(void)\n {\n \tstruct branch *current_branch = branch_get(NULL);\n@@ -857,7 +840,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tstruct string_list strategy_options = STRING_LIST_INIT_NODUP;\n \tstruct object_id squash_onto;\n \tchar *squash_onto_name = NULL;\n-\tstruct opt_y opt_y = { .list = &exec, .options = &options };\n \tstruct option builtin_rebase_options[] = {\n \t\tOPT_STRING(0, \"onto\", &options.onto_name,\n \t\t\t N_(\"revision\"),\n@@ -935,9 +917,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST('x', \"exec\", &exec, N_(\"exec\"),\n \t\t\t\tN_(\"add exec lines after each commit of the \"\n \t\t\t\t \"editable list\")),\n-\t\t{ OPTION_CALLBACK, 'y', NULL, &opt_y, N_(\"\"),\n-\t\t\tN_(\"same as --reschedule-failed-exec -x \"),\n-\t\t\tPARSE_OPT_NONEG, parse_opt_y },\n \t\tOPT_BOOL(0, \"allow-empty-message\",\n \t\t\t &options.allow_empty_message,\n \t\t\t N_(\"allow rebasing commits with empty messages\")),\ndiff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh\nindex 3bb0682db5..37db5a7ca4 100755\n--- a/git-legacy-rebase.sh\n+++ b/git-legacy-rebase.sh\n@@ -26,7 +26,6 @@ f,force-rebase! cherry-pick all commits, even if unchanged\n m,merge! use merging strategies to rebase\n i,interactive! let the user edit the list of commits to rebase\n x,exec=! add exec lines after each commit of the editable list\n-y=! same as --reschedule-failed-exec -x\n k,keep-empty\t preserve empty commits during rebase\n allow-empty-message allow rebasing commits with empty messages\n stat! display a diffstat of what changed upstream\n@@ -263,11 +262,6 @@ do\n \t\tcmd=\"${cmd}exec ${1#--exec=}${LF}\"\n \t\ttest -z \"$interactive_rebase\" && interactive_rebase=implied\n \t\t;;\n-\t-y*)\n-\t\treschedule_failed_exec=--reschedule-failed-exec\n-\t\tcmd=\"${cmd}exec ${1#-y}${LF}\"\n-\t\ttest -z \"$interactive_rebase\" && interactive_rebase=implied\n-\t\t;;\n \t--interactive)\n \t\tinteractive_rebase=explicit\n \t\t;;\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 25aaacacfc..bdaa511bb0 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -262,9 +262,6 @@ test_expect_success '--reschedule-failed-exec' '\n \ttest_must_fail git -c rebase.rescheduleFailedExec=true \\\n \t\trebase -x false HEAD^ 2>err &&\n \tgrep \"^exec false\" .git/rebase-merge/git-rebase-todo &&\n-\ttest_i18ngrep \"has been rescheduled\" err &&\n-\tgit rebase --abort &&\n-\ttest_must_fail git rebase -y false HEAD^ 2>err &&\n \ttest_i18ngrep \"has been rescheduled\" err\n '\n \n-- \ngitgitgadget\n"},{"id":"368530","messageId":"xmqq8syut9op.fsf@gitster-ct.c.googlers.com","threadId":"50404","inReplyTo":"e61ebc30605e21ce71623903bc9c850fd964e826.1549367342.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] Revert \"rebase: introduce a shortcut for --reschedule-failed-exec\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-05T17:41:10Z","receivedAt":"2019-02-05T17:41:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> When contributing the patch series, the cover letter tried to convey\n> clearly that the patch introducing the shortcut -y was included only to\n> show that it is possible, with a slight bias against it.\n>\n> During the review, there were a couple reviewers who agreed with this\n> sentiment, and the author was happy that this patch was not needed and\n> concurred that it should be dropped. See e.g. Stefan Beller's reply:\n> \n>\n> However, it slipped by the original patch author (yours truly) that the\n> patch *was* included when the branch made it to `next` and then when it\n> made it to `master`.\n>\n> So let's back out that patch before it even slips into an official\n> release (in which case we would even have to support this unwanted\n> flag).\n>\n> This reverts commit 81ef8ee75d5f348d3c71ff633d13d302124e1a5e.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\nThanks for catching before feature freeze, but read the above again\nwith cooler head. The revert message is less useful than if you\nsaid \n\n The patch was sent for completeness just in case it turns out to\n be too cumbersome not to have a short-hand option, but during\n the discussion, reviewers agreed that [FOR SUCH AND SUCH REASONS\n --- fill in the blank here] we are better off without. The\n maintainer missed that conclusion and forgot to drop it while\n merging the topic down, and contributors did not notice the\n mistake, either.\n\nAs the reason is missing, the only thing a reader can get from it is\n\"the patch was not intended to be included, but we screwed up\". I\ndo not see why a more useful \"why it wasn't intended to be included\"\nneeds to be hidden behind an external reference.\n\n"},{"id":"368624","messageId":"pull.118.v2.git.gitgitgadget@gmail.com","threadId":"50404","inReplyTo":"pull.118.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] rebase: drop the unwanted -y","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-02-06T18:45:17Z","receivedAt":"2019-02-06T18:45:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I totally missed that this patch made it into next, and then master. Sorry\nabout that.\n\nChanges since v1:\n\n * Focused on the \"why?\" in the commit message.\n\nJohannes Schindelin (1):\n Revert \"rebase: introduce a shortcut for --reschedule-failed-exec\"\n\n Documentation/git-rebase.txt | 6 ------\n builtin/rebase.c | 21 ---------------------\n git-legacy-rebase.sh | 6 ------\n t/t3418-rebase-continue.sh | 3 ---\n 4 files changed, 36 deletions(-)\n\n\nbase-commit: b5101f929789889c2e536d915698f58d5c5c6b7a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-118%2Fdscho%2Frevert-rebase-y-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-118/dscho/revert-rebase-y-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/118\n\nRange-diff vs v1:\n\n 1: e61ebc3060 ! 1: a4857fb32d Revert \"rebase: introduce a shortcut for --reschedule-failed-exec\"\n @@ -2,22 +2,20 @@\n \n Revert \"rebase: introduce a shortcut for --reschedule-failed-exec\"\n \n - When contributing the patch series, the cover letter tried to convey\n - clearly that the patch introducing the shortcut -y was included only to\n - show that it is possible, with a slight bias against it.\n + This patch was contributed only as a tentative \"we could introduce a\n + convenient short option if we do not want to change the default behavior\n + in the long run\" patch, opening the discussion whether other people\n + agree with deprecating the current behavior in favor of the rescheduling\n + behavior.\n \n - During the review, there were a couple reviewers who agreed with this\n - sentiment, and the author was happy that this patch was not needed and\n - concurred that it should be dropped. See e.g. Stefan Beller's reply:\n + But the consensus on the Git mailing list was that it would make sense\n + to show a warning in the near future, and flip the default\n + rebase.rescheduleFailedExec to reschedule failed `exec` commands by\n + default. See e.g.\n \n \n - However, it slipped by the original patch author (yours truly) that the\n - patch *was* included when the branch made it to `next` and then when it\n - made it to `master`.\n -\n - So let's back out that patch before it even slips into an official\n - release (in which case we would even have to support this unwanted\n - flag).\n + So let's back out that patch that added the `-y` short option that we\n + agreed was not necessary or desirable.\n \n This reverts commit 81ef8ee75d5f348d3c71ff633d13d302124e1a5e.\n \n\n-- \ngitgitgadget\n"},{"id":"368625","messageId":"a4857fb32d99088f27b5a80d23c4f9ec15586396.1549478715.git.gitgitgadget@gmail.com","threadId":"50404","inReplyTo":"pull.118.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] Revert \"rebase: introduce a shortcut for --reschedule-failed-exec\"","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-02-06T18:45:17Z","receivedAt":"2019-02-06T18:45:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis patch was contributed only as a tentative \"we could introduce a\nconvenient short option if we do not want to change the default behavior\nin the long run\" patch, opening the discussion whether other people\nagree with deprecating the current behavior in favor of the rescheduling\nbehavior.\n\nBut the consensus on the Git mailing list was that it would make sense\nto show a warning in the near future, and flip the default\nrebase.rescheduleFailedExec to reschedule failed `exec` commands by\ndefault. See e.g.\n\n\nSo let's back out that patch that added the `-y` short option that we\nagreed was not necessary or desirable.\n\nThis reverts commit 81ef8ee75d5f348d3c71ff633d13d302124e1a5e.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-rebase.txt | 6 ------\n builtin/rebase.c | 21 ---------------------\n git-legacy-rebase.sh | 6 ------\n t/t3418-rebase-continue.sh | 3 ---\n 4 files changed, 36 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 4dd5853d6e..44ffe2c8c5 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -462,12 +462,6 @@ without an explicit `--interactive`.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n--y ::\n-\tThis is the same as passing `--reschedule-failed-exec` before\n-\t`-x `, i.e. it appends the specified `exec` command and\n-\tturns on the mode where failed `exec` commands are automatically\n-\trescheduled.\n-\n --root::\n \tRebase all commits reachable from , instead of\n \tlimiting them with an . This allows you to rebase\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 774264bae8..7c77a80687 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -760,23 +760,6 @@ static int parse_opt_interactive(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-struct opt_y {\n-\tstruct string_list *list;\n-\tstruct rebase_options *options;\n-};\n-\n-static int parse_opt_y(const struct option *opt, const char *arg, int unset)\n-{\n-\tstruct opt_y *o = opt->value;\n-\n-\tif (unset || !arg)\n-\t\treturn -1;\n-\n-\to->options->reschedule_failed_exec = 1;\n-\tstring_list_append(o->list, arg);\n-\treturn 0;\n-}\n-\n static void NORETURN error_on_missing_default_upstream(void)\n {\n \tstruct branch *current_branch = branch_get(NULL);\n@@ -857,7 +840,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tstruct string_list strategy_options = STRING_LIST_INIT_NODUP;\n \tstruct object_id squash_onto;\n \tchar *squash_onto_name = NULL;\n-\tstruct opt_y opt_y = { .list = &exec, .options = &options };\n \tstruct option builtin_rebase_options[] = {\n \t\tOPT_STRING(0, \"onto\", &options.onto_name,\n \t\t\t N_(\"revision\"),\n@@ -935,9 +917,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST('x', \"exec\", &exec, N_(\"exec\"),\n \t\t\t\tN_(\"add exec lines after each commit of the \"\n \t\t\t\t \"editable list\")),\n-\t\t{ OPTION_CALLBACK, 'y', NULL, &opt_y, N_(\"\"),\n-\t\t\tN_(\"same as --reschedule-failed-exec -x \"),\n-\t\t\tPARSE_OPT_NONEG, parse_opt_y },\n \t\tOPT_BOOL(0, \"allow-empty-message\",\n \t\t\t &options.allow_empty_message,\n \t\t\t N_(\"allow rebasing commits with empty messages\")),\ndiff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh\nindex 3bb0682db5..37db5a7ca4 100755\n--- a/git-legacy-rebase.sh\n+++ b/git-legacy-rebase.sh\n@@ -26,7 +26,6 @@ f,force-rebase! cherry-pick all commits, even if unchanged\n m,merge! use merging strategies to rebase\n i,interactive! let the user edit the list of commits to rebase\n x,exec=! add exec lines after each commit of the editable list\n-y=! same as --reschedule-failed-exec -x\n k,keep-empty\t preserve empty commits during rebase\n allow-empty-message allow rebasing commits with empty messages\n stat! display a diffstat of what changed upstream\n@@ -263,11 +262,6 @@ do\n \t\tcmd=\"${cmd}exec ${1#--exec=}${LF}\"\n \t\ttest -z \"$interactive_rebase\" && interactive_rebase=implied\n \t\t;;\n-\t-y*)\n-\t\treschedule_failed_exec=--reschedule-failed-exec\n-\t\tcmd=\"${cmd}exec ${1#-y}${LF}\"\n-\t\ttest -z \"$interactive_rebase\" && interactive_rebase=implied\n-\t\t;;\n \t--interactive)\n \t\tinteractive_rebase=explicit\n \t\t;;\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 25aaacacfc..bdaa511bb0 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -262,9 +262,6 @@ test_expect_success '--reschedule-failed-exec' '\n \ttest_must_fail git -c rebase.rescheduleFailedExec=true \\\n \t\trebase -x false HEAD^ 2>err &&\n \tgrep \"^exec false\" .git/rebase-merge/git-rebase-todo &&\n-\ttest_i18ngrep \"has been rescheduled\" err &&\n-\tgit rebase --abort &&\n-\ttest_must_fail git rebase -y false HEAD^ 2>err &&\n \ttest_i18ngrep \"has been rescheduled\" err\n '\n \n-- \ngitgitgadget\n"},{"id":"368634","messageId":"xmqq4l9gra16.fsf@gitster-ct.c.googlers.com","threadId":"50404","inReplyTo":"pull.118.v2.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/1] rebase: drop the unwanted -y","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-06T19:28:53Z","receivedAt":"2019-02-06T19:28:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> I totally missed that this patch made it into next, and then master. Sorry\n> about that.\n\nWill queue. Let's make sure -y won't escape to the released\nversion(s).\n\nThanks for a quick update. \n\n"}]}