Re: [PATCH v9 0/3] rebase: document, clean up, and introduce a config option for --rebase-merges

[Phillip Wood <phillip.wood123@xxxxxxxxx>]

Hi Alex

On 26/03/2023 04:06, Alex Henrie wrote:
> This patch series introduces a rebase.rebaseMerges config option to
> accommodate users who would like --rebase-merges to be on by default and
> to facilitate turning on --rebase-merges by default without
> configuration in a future version of Git. It also cleans up and
> documents the behavior of the --rebase-merges command line option to
> avoid confusion about how the config option and the command line option
> interact.
>
> Changes from v8:
> - Add braces around one-line else clause
> - Remove unnecessary change to error message priority

The range-diff looks good to me. This iteration addresses all of my 
outstanding concerns.

Thanks

Phillip

> Thanks to Phillip, Junio, Johannes and Sergey for your feedback on v8.
>
> Alex Henrie (3):
>    rebase: add documentation and test for --no-rebase-merges
>    rebase: deprecate --rebase-merges=""
>    rebase: add a config option for --rebase-merges
>
>   Documentation/config/rebase.txt        | 10 ++++
>   Documentation/git-rebase.txt           | 19 ++++---
>   builtin/rebase.c                       | 70 ++++++++++++++++++++------
>   t/t3422-rebase-incompatible-options.sh | 17 +++++++
>   t/t3430-rebase-merges.sh               | 44 ++++++++++++++++
>   5 files changed, 138 insertions(+), 22 deletions(-)
>
> Range-diff against v8:
> 1:  09fb7c1b74 = 1:  a22b9d0da2 rebase: add documentation and test for --no-rebase-merges
> 2:  a846716a4a = 2:  112fee4833 rebase: deprecate --rebase-merges=""
> 3:  b12a3610ba ! 3:  868899cd6d rebase: add a config option for --rebase-merges
>      @@ builtin/rebase.c: static int rebase_config(const char *var, const char *value, v
>       +		if (opts->config_rebase_merges < 0) {
>       +			opts->config_rebase_merges = 1;
>       +			parse_rebase_merges_value(opts, value);
>      -+		} else
>      ++		} else {
>       +			opts->rebase_cousins = 0;
>      ++		}
>       +		return 0;
>       +	}
>       +
>      @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix
>        		if (ignore_whitespace)
>        			strvec_push(&options.git_am_opts,
>       @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
>      - 				break;
>      -
>      - 		if (i >= 0 || options.type == REBASE_APPLY) {
>      --			if (is_merge(&options))
>      --				die(_("apply options and merge options "
>      --					  "cannot be used together"));
>      --			else if (options.autosquash == -1 && options.config_autosquash == 1)
>      -+			if (options.autosquash == -1 && options.config_autosquash == 1)
>      + 					  "cannot be used together"));
>      + 			else if (options.autosquash == -1 && options.config_autosquash == 1)
>        				die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
>       +			else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
>       +				die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
>        			else if (options.update_refs == -1 && options.config_update_refs == 1)
>        				die(_("apply options are incompatible with rebase.updateRefs.  Consider adding --no-update-refs"));
>      -+			else if (is_merge(&options))
>      -+				die(_("apply options and merge options "
>      -+					  "cannot be used together"));
>        			else
>      - 				options.type = REBASE_APPLY;
>      - 		}
>       @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
>        	options.update_refs = (options.update_refs >= 0) ? options.update_refs :
>        			     ((options.config_update_refs >= 0) ? options.config_update_refs : 0);