From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 06 Jul 2026 13:40:07 GMT
Subject: Re: [PATCH 08/11] sequencer: simplify pick_one_commit()
Message-ID: <5d112698-75be-4b44-a3d9-8b6ecb4924de@gmail.com>
In-Reply-To: <akuMQ45aQejRcQ_Y@ugly.lan>



On 06/07/2026 12:06, Oswald Buddenhagen wrote:
> On Tue, Jun 30, 2026 at 04:28:58PM +0100, Phillip Wood wrote:
>> +++ b/sequencer.c
>> @@ -4981,14 +4983,13 @@ static int pick_one_commit(struct repository *r,
>>         }
>>         return error_with_patch(r, commit,
>>                     arg, item->arg_len, opts, res, !res);
>> -    }
>> -    if (is_rebase_i(opts) && !res)
>> +    } else if (!res) {
>>
> because of this ...
> 
>>         record_in_rewritten(&item->commit->object.oid,
>>                     peek_command(todo_list, 1));
>> -    if (res && is_fixup(item->command)) {
>> +    } else if (res && is_fixup(item->command)) {
>>
> .. the res conditional is pointless here.
> 
>>         return error_failed_squash(r, item->commit, opts,
>>                        item->arg_len, arg);
>> -    } else if (res && is_rebase_i(opts)) {
>> +    } else if (res) {
>>
> and here as well.

I meant to add a comment about that to the commit message. I 
deliberately left them alone so that when we convert them to use the 
enum it is clear that these arms are handling cases with conflicts.

Thanks

Phillip