Subject: Re: [PATCH 1/8] rebase: simplify code related to imply_merge()

From: Junio C Hamano <[email-masked]>
Date: Thu, 23 Mar 2023 13:00:53 -0700

Phillip Wood <[email-masked]> writes:

> On 23/03/2023 16:22, Oswald Buddenhagen wrote:
>> The code's evolution left in some bits surrounding enum rebase_type that
>> don't really make sense any more. In particular, it makes no sense to
>> invoke imply_merge() if the type is already known not to be
>> REBASE_APPLY, and it makes no sense to assign the type after calling
>> imply_merge().
>
> These look sensible, did imply_merges() use to do something more which
> made these calls useful?

Good question.

>>   @@ -1494,9 +1493,6 @@ int cmd_rebase(int argc, const char **argv,
>> const char *prefix)
>>   		}
>>   	}
>>   -	if (options.type == REBASE_MERGE)
>> -		imply_merge(&options, "--merge");

This piece is reasonable, of course.  We already know we are in
merge mode so there is nothing implied.

Before this hunk, there is a bit of code to react to
options.strategy given.  The code complains if we are using the
apply backend, and sets the options.type to REBASE_MERGE, which is
suspiciously similar to what imply_merge() is doing.  I wonder if
the code should be simplified to make a call to imply_merge() while
we are doing similar simplification like this patch does?