Re: [PATCH v6 3/3] rebase: add a config option for --rebase-merges

From: Glen Choo <chooglen@google.com>
Date: Mar 16, 2023, 17:32 UTC
Message-ID: <kl6lzg8cvby2.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <CAMMLpeRGEETraueJTTV0tJSsycNYF24YX8n6h-pMp87VcCRJtQ@mail.gmail.com>

Alex Henrie <alexhenrie24@gmail.com> writes:

Show 14 quoted lines

>> > In the future, the default rebase-merges mode may change from
>> > no-rebase-cousins to rebase-cousins.
>>
>> I suspect a more likely future would be that the default is to rebase
>> 'evil' merges instead of trying to recreate merge commits, but of
>> course, the important thing is that we promote the default, not what the
>> default will be...
>
> Glen, do you have any more thoughts? At this point, the only thing
> that's keeping me from implementing Phillip's request to make
> --rebase-merges without an argument clobber rebase.rebaseMerges is
> your suspicion that we might need to change the default rebase-merges
> mode in the future, and I assume that we would want to use the
> rebase.rebaseMerges config option to facilitate the transition.

(Sorry for the late reply)

Ah, I don't really have more thoughts on the matter. I am fairly confident that we would _like_ to change the default to rebase 'evil' merges, but I don't know how likely that will be.

Perhaps it would help to enumerate the rules to see if it is too confusing or not?

The behaviors we can tweak are:

- Whether to rebase merges or not (true, false, specified mode, or
  default)
- What mode to use when rebasing merges (specified mode or default)

And the sources are either CLI or config, with CLI always overriding config.

Should we rebase a merge?

- If neither CLI or config tells us whether or not to rebase a merge,
  default to "don't rebase merges".
- If one of CLI or config tells us whether or not to rebase a merge,
  respect it.
- If both CLI or config tell us whether or not to rebase a merge,
  respect CLI and ignore config.

What mode should we use?

- If neither CLI or config tells us what mode to use, default to
  "no-rebase-cousins" (or whatever default we decide).
- If one of CLI or config tells us what mode to use, respect it.
- If both CLI or config tell us what mode to use, respect CLI and ignore
  config.

If users cleanly separate the two concepts, I think it is quite clear. (I'm not advocating for this approach, but) e.g. if we pretend that each behavior were configured separately, like:

--[no-]rebase-merges [--rebase-merges-mode=(rebase-cousins|no-rebase-cousins)]

I don't think there would be any confusion. (Having --rebase-merges-mode be a no-op without --rebase-merges is probably even more confusing to users, plus this would break backwards compatibility, so I don't think this is a good idea at all.)

Your doc patch explains the rules pretty clearly, but perhaps it doesn't explain this mental model clearly enough, hence the confusion. If we don't find a good way to communicate this (I think it is clear, but other reviewers seem yet unconvinced), I wouldn't mind taking Phillip's suggestion to have "--rebase-merges" override "rebase.rebaseMerges='specific-mode'".

>
> -Alex