diff mbox series

[v5,1/3] rebase: add documentation and test for --no-rebase-merges

Message ID 20230225180325.796624-2-alexhenrie24@gmail.com (mailing list archive)
State New, archived
Headers show
Series rebase: add a config option for --rebase-merges | expand

Commit Message

Alex Henrie Feb. 25, 2023, 6:03 p.m. UTC
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/git-rebase.txt |  4 +++-
 t/t3430-rebase-merges.sh     | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Glen Choo March 1, 2023, 11:23 p.m. UTC | #1
Alex Henrie <alexhenrie24@gmail.com> writes:

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

I inferred from the 'subject line' that this "always" worked and was
undocumented, and that we'd want to document this not for its own sake,
but because we'll introduce a config option. I would personally prefer
to get that explicitly in writing, but it's definitely not worth a
reroll.

>  -r::
>  --rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
> +--no-rebase-merges::
>  	By default, a rebase will simply drop merge commits from the todo
>  	list, and put the rebased commits into a single, linear branch.
>  	With `--rebase-merges`, the rebase will instead try to preserve
>  	the branching structure within the commits that are to be rebased,
>  	by recreating the merge commits. Any resolved merge conflicts or
>  	manual amendments in these merge commits will have to be
> -	resolved/re-applied manually.
> +	resolved/re-applied manually. `--no-rebase-merges` can be used to
> +	countermand a previous `--rebase-merges`.
>  +
>  By default, or when `no-rebase-cousins` was specified, commits which do not
>  have `<upstream>` as direct ancestor will keep their original branch point,
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh

This isn't the fault of this patch, but I find this description a little
confusing. First we say

  By default (*when no option is given*), a rebase will simply drop
  merge commits from the todo list...

and then later we also say

  By default (*when --rebase-merges is provided without a value), or
  when `no-rebase-cousins`...

So although you didn't create this problem, given the changes we're
making in the later patches, I think we have the chance to make this
description more sensible; I'll elaborate in my response to those
patches.
diff mbox series

Patch

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9a295bcee4..c98784a0d2 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -529,13 +529,15 @@  See also INCOMPATIBLE OPTIONS below.
 
 -r::
 --rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
+--no-rebase-merges::
 	By default, a rebase will simply drop merge commits from the todo
 	list, and put the rebased commits into a single, linear branch.
 	With `--rebase-merges`, the rebase will instead try to preserve
 	the branching structure within the commits that are to be rebased,
 	by recreating the merge commits. Any resolved merge conflicts or
 	manual amendments in these merge commits will have to be
-	resolved/re-applied manually.
+	resolved/re-applied manually. `--no-rebase-merges` can be used to
+	countermand a previous `--rebase-merges`.
 +
 By default, or when `no-rebase-cousins` was specified, commits which do not
 have `<upstream>` as direct ancestor will keep their original branch point,
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index fa2a06c19f..d46d9545f2 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -250,6 +250,16 @@  test_expect_success 'with a branch tip that was cherry-picked already' '
 	EOF
 '
 
+test_expect_success '--no-rebase-merges countermands --rebase-merges' '
+	git checkout -b no-rebase-merges E &&
+	git rebase --rebase-merges --no-rebase-merges C &&
+	test_cmp_graph C.. <<-\EOF
+	* B
+	* D
+	o C
+	EOF
+'
+
 test_expect_success 'do not rebase cousins unless asked for' '
 	git checkout -b cousins main &&
 	before="$(git rev-parse --verify HEAD)" &&