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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Mar 16, 2023, 23:48 UTC
Message-ID: <CAMP44s04zfDjXRpof5ZBTbTLD5aq5_rQc_74ipDv1N-d-dhtKw@mail.gmail.com>
In-Reply-To: <kl6lttykuxfu.fsf@chooglen-macbookpro.roam.corp.google.com>

On Thu, Mar 16, 2023 at 4:46 PM Glen Choo <chooglen@google.com> wrote:

Show 19 quoted lines

>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> >> --[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.)
> >
> > I don't find it confusing. And how would it break backwards
> > compatibility if --rebase-merges-mode doesn't exist now?
>
> I meant that for the above example to work, we would need to have
> `--[no-]rebase-merges` as a boolean that says whether we rebase merges at
> all, and `--rebase-merges-mode=[whatever]` would tell us what mode to
> use _if_ we were rebasing merges.

No, we don't need --no-rebase-merges for --rebase-merges-mode to work. It would be nice, but not necessary.

> This means that
> `--rebase-merges=not-a-boolean` would become invalid.

No, it would not become invalid, that's yet another decision to make. `--rebase-merges=mode` could become synonymous with `--rebase-merges --rebase-merges-mode=mode`. A shortcut.

Because it's redundant it could be deprecated in the future, but not necessarily invalid.

> We were in this position before, actually, with `git log -p -m`. The
> conclusion on a recent series [1] is that having such a no-op flag was
> probably a mistake (and unfortunately, one that is extremely hard to fix
> due to backwards compatibility requirements),

No, `git log -m` was a mistake *only* if -p isn't turned on as well, which is an interface wart that could be fixed today.

> so introducing more no-op flags is probably a bad idea :)

Whether --rebase-merges-mode turns on --rebase-merges by default is a decision that could be made in the future. Just like `git log -m` turning on `-p` by default is a decision that could be made in the future (and probably should).

---

Either way: introducing a new option cannot possibly break backwards compatibility.

Cheers.

-- 
Felipe Contreras