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

From: Alex Henrie <alexhenrie24@gmail.com>
Date: Feb 20, 2023, 17:06 UTC
Message-ID: <CAMMLpeQ8_Wz7sEE9M1t6oLF_BA7T_BT9TNfkKwgGOvf9fiio2Q@mail.gmail.com>
In-Reply-To: <fe9a3c86-0169-588f-2b12-e124d9d138d9@dunelm.org.uk>

On Mon, Feb 20, 2023 at 2:38 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 6 quoted lines

>
> On 20/02/2023 03:32, Alex Henrie wrote:
>
> I think the commit message could benefit from some justification for why
> this config option is useful. I don't object to it being added but you
> need to make the case for why it is a good idea.

The purpose of the new option is to accommodate users who would like --rebase-merges to be on by default and to facilitate possibly turning on --rebase-merges by default without configuration in a future version of Git. I'll add a note about that to the config message.

Show 6 quoted lines

> > At the same time, stop accepting --rebase-merges="" as a synonym of
> > --rebase-merges=no-rebase-cousins.
>
> Please try to avoid combining unrelated changes in the same patch. I
> agree that accepting an empty argument to mean "no-rebase-cousins" is
> slightly odd but as that is the default I'm not sure it is doing any harm.

I wrote the code so that `git config rebase.merges ""` has the same effect on `git rebase` as `git config --unset rebase.merges`, because I think that's what most people are going to expect. I'd like to get rid of the odd syntax --rebase-merges="" because a user might reasonably expect it to do the same thing as `git config rebase.merges ""`, but it doesn't. On top of that, the config option uses the same helper function as the command-line option. So I consider removing --rebase-merges="" to be intertwined with adding the config option, but I'll split them into separate patches anyway.

Thanks for the feedback,

-Alex