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

[Alex Henrie <alexhenrie24@gmail.com>]

On Wed, Mar 22, 2023 at 10:54 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

> Hurray! Thanks for re-rolling.

Thanks for making sure that we got the UI right!

> On 20/03/2023 05:59, Alex Henrie wrote:

> > +     if (!strcmp(var, "rebase.rebasemerges")) {
> > +             opts->config_rebase_merges = git_parse_maybe_bool(value);
> > +             if (opts->config_rebase_merges < 0) {
> > +                     opts->config_rebase_merges = 1;
> > +                     parse_rebase_merges_value(opts, value);
> > +             } else
> > +                     opts->rebase_cousins = 0;
>
> The "else" clause should have braces because the "if" cause requires
> them (see Documentation/CodingGuidelines). I don't think it is worth
> re-rolling just for this though.

Thanks for pointing out that documentation. I'm going to make a v9
anyway, and I'll add the braces then. By the way, actions speak louder
than words: While writing the patch, I found several examples of the
braces being omitted in cases like this in other places in rebase.c,
so I assumed that that was the preferred style here. If you want to
encourage people to follow the CodingGuidelines document, the best way
would be to clean up the existing code to conform to it.

-Alex