Re: [PATCH v2 3/6] sequencer: factor out part of pick_commits()

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 21, 2023, 21:21 UTC
Message-ID: <xmqqleilhqxd.fsf@gitster.g>
In-Reply-To: <049a792d-e015-3583-452d-923b9aee4a72@gmail.com>

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

Show 6 quoted lines

> There are two blocks that might be entered. One guarded by "if
> (reschedule)" - this is not entered because reschedlue is always zero
> when picking a commit. The other is guarded by "else if
> (is_rebase_i(opts) && check_todo && !res)" and so will not be entered
> when we want to return an error because "res" is non-zero in that
> case.

Perhaps the proposed log message can be updated to mention these to save time from "git log" readers?

>>   * the todo_list->current counter is incremented

The todo_list->current counter gets incremented before leaving the function now, but all three callers of pick_commits() immediately call todo_list_release(), the value, whether it is incremented or not, is discarded without getting looked at.

So it is not a noop, but the difference in behaviour does not become externally observable?

Thanks.