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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 23, 2023, 20:00 UTC
Message-ID: <xmqqiler8cga.fsf@gitster.g>
In-Reply-To: <2b296b75-3f8d-28a9-a3d8-8134450852da@dunelm.org.uk>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 9 quoted lines

> 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.

Show 6 quoted lines

>>   @@ -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?