From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 20 Feb 2023 09:31:30 GMT
Subject: Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option
Message-ID: <8857003a-f33d-00c5-5e1e-bfbcd60924ae@dunelm.org.uk>
In-Reply-To: <20230220033224.10400-1-alexhenrie24@gmail.com>

Hi Alex

On 20/02/2023 03:32, Alex Henrie wrote:
> Name the new option "drop" intead of "no" or "false" to avoid confusion > in the future if --rebase-merges grows the ability to truly "rebase"
> merge commits by reusing the conflict resolution information from the
> original merge commit, and we want to add an option to ignore the
> conflict resolution information.
> 
> This option can be used to countermand a previous --rebase-merges
> option.

I'm a bit confused as to the reason for this change - what's the 
advantage over just saying --no-rebase-merges which already exists?

Best Wishes

Phillip

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>   Documentation/git-rebase.txt |  2 +-
>   builtin/rebase.c             |  2 +-
>   t/t3430-rebase-merges.sh     | 30 ++++++++++++++++++++++++++++++
>   3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 9a295bcee4..92e90f96aa 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -528,7 +528,7 @@ have the long commit hash prepended to the format.
>   See also INCOMPATIBLE OPTIONS below.
>   
>   -r::
> ---rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
> +--rebase-merges[=(rebase-cousins|no-rebase-cousins|drop)]::
>   	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
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 6635f10d52..96c0474379 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1436,7 +1436,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	if (options.exec.nr)
>   		imply_merge(&options, "--exec");
>   
> -	if (rebase_merges) {
> +	if (rebase_merges && strcmp("drop", rebase_merges)) {
>   		if (!*rebase_merges)
>   			; /* default mode; do nothing */
>   		else if (!strcmp("rebase-cousins", rebase_merges))
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
> index fa2a06c19f..861c8405f2 100755
> --- a/t/t3430-rebase-merges.sh
> +++ b/t/t3430-rebase-merges.sh
> @@ -250,6 +250,36 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
>   	EOF
>   '
>   
> +test_expect_success 'do not rebase merges unless asked' '
> +	git checkout -b rebase-merges-default E &&
> +	before="$(git rev-parse --verify HEAD)" &&
> +	test_tick &&
> +	git rebase --rebase-merges C &&
> +	test_cmp_rev HEAD $before &&
> +	test_tick &&
> +	git rebase C &&
> +	test_cmp_graph C.. <<-\EOF
> +	* B
> +	* D
> +	o C
> +	EOF
> +'
> +
> +test_expect_success 'do not rebase merges when asked to drop them' '
> +	git checkout -b rebase-merges-drop E &&
> +	before="$(git rev-parse --verify HEAD)" &&
> +	test_tick &&
> +	git rebase --rebase-merges C &&
> +	test_cmp_rev HEAD $before &&
> +	test_tick &&
> +	git rebase --rebase-merges=drop 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)" &&