Subject: Re: [PATCH 3/5] rebase: factor out merge_base calculation

From: Junio C Hamano <[email-masked]>
Date: Tue, 16 Aug 2022 08:00:44 -0700

Johannes Schindelin <[email-masked]> writes:

> Hi Junio,
>
> On Mon, 15 Aug 2022, Junio C Hamano wrote:
>
>> "Phillip Wood via GitGitGadget" <[email-masked]> writes:
>>
>> > From: Phillip Wood <[email-masked]>
>> >
>> > Separate out calculating the merge base between onto and head from the
>> > check for whether we can fast-forward or not. This means we can skip
>> > the fast-forward checks when the rebase is forced and avoid
>> > calculating the merge-base twice when --keep-base is given.
>> >
>> > Signed-off-by: Phillip Wood <[email-masked]>
>> > ---
>> > Note the unnecessary braces around "if (keep_base)" are added here
>> > reduce the churn on the next commit.
>
> This note...
>
>> > @@ -1668,7 +1678,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>> >  			die(_("Does not point to a valid commit '%s'"),
>> >  				options.onto_name);
>> >  	}
>> > -
>> > +	if (keep_base) {
>> > +		oidcpy(&merge_base, &options.onto->object.oid);
>> > +	} else {
>> > +		fill_merge_base(&options, &merge_base);
>> > +	}
>>
>> No need for braces around single-statement block on either side.
>
> ... already addresses this feedback.

Yeah, but the point is instead of wasting readers' bandwidth with an
additional note, the series can add braces in the step where they
become necessary, i.e. the later step where there is a new statement
in the body of the "if-true" side.