Re: [PATCH v5 2/3] rebase: deprecate --rebase-merges=""
From: Phillip Wood <hidden>
Date: 2023-03-02 10:08:20
Hi Alex On 25/02/2023 18:03, Alex Henrie wrote:
The unusual syntax --rebase-merges="" (that is, --rebase-merges with an empty string argument) has been an undocumented synonym of --rebase-merges=no-rebase-cousins. Deprecate that syntax to avoid confusion when a rebase.merges config option is introduced, where rebase.merges="" will be equivalent to --no-rebase-merges.
I think deprecating this rather than making it an error is a good idea. I don't think we need the test though. The test suite is incredibly slow to run on windows (I'd heard people complain about it but it was not until I tried running it myself I realized just how diabolically slow it really is) and so it is important to balance adding tests to catch regression vs test run time. Tests that catch bugs in the rebase implementation are useful, ones like this just make everything slower which makes it harder to catch real regressions. Best Wishes Phillip
quoted hunk ↗ jump to hunk
Signed-off-by: Alex Henrie <redacted> --- builtin/rebase.c | 8 ++++++-- t/t3430-rebase-merges.sh | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-)diff --git a/builtin/rebase.c b/builtin/rebase.c index 6635f10d52..ccc13200b5 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c@@ -1140,7 +1140,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) {OPTION_STRING, 'r', "rebase-merges", &rebase_merges, N_("mode"), N_("try to rebase merges instead of skipping them"), - PARSE_OPT_OPTARG, NULL, (intptr_t)""}, + PARSE_OPT_OPTARG, NULL, (intptr_t)"no-rebase-cousins"}, OPT_BOOL(0, "fork-point", &options.fork_point, N_("use 'merge-base --fork-point' to refine upstream")), OPT_STRING('s', "strategy", &options.strategy,@@ -1438,7 +1438,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) if (rebase_merges) { if (!*rebase_merges) - ; /* default mode; do nothing */ + warning(_("--rebase-merges with an empty string " + "argument is deprecated and will stop " + "working in a future version of Git. Use " + "--rebase-merges=no-rebase-cousins " + "instead.")); else if (!strcmp("rebase-cousins", rebase_merges)) options.rebase_cousins = 1; else if (strcmp("no-rebase-cousins", rebase_merges))diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index d46d9545f2..f50fbf1390 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh@@ -278,6 +278,11 @@ test_expect_success 'do not rebase cousins unless asked for' ' EOF ' +test_expect_success '--rebase-merges="" is deprecated' ' + git rebase --rebase-merges="" HEAD^ 2>actual && + grep deprecated actual +' + test_expect_success 'refs/rewritten/* is worktree-local' ' git worktree add wt && cat >wt/script-from-scratch <<-\EOF &&