Re: [RFC PATCH 4/9] sequencer: update `done_nr' when skipping commands in a todo list

From: Junio C Hamano <gitster@pobox.com>
To: Alban Gruin <alban.gruin@gmail.com>
Cc: git@vger.kernel.org,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [RFC PATCH 4/9] sequencer: update `done_nr' when skipping commands in a todo list
Date: Thu, 18 Jul 2019 12:55:12 -0700	[thread overview]
Message-ID: <xmqqo91rxgm7.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190717143918.7406-5-alban.gruin@gmail.com> (Alban Gruin's message of "Wed, 17 Jul 2019 16:39:13 +0200")

Alban Gruin <alban.gruin@gmail.com> writes:

> In a todo list, `done_nr' is the amount of commands that were executed
> or skipped, but skip_unnecessary_picks() did not update it.

OK.  Together with 3/9 and this one, any increment of total_nr and
done_nr in the existing code is not removed; does it mean that
nobody actually cares what these fields contain?  IOW, there is no
code that says "if (list->total_nr <= i) { we are done; }" etc.?

Or are these fields used later, but somehow the lack of increment in
the places touched by 3/9 and 4/9 is compensated?

> Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
> ---
>  sequencer.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/sequencer.c b/sequencer.c
> index e61ae75451..ec9c3d4dc5 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4939,6 +4939,7 @@ static int skip_unnecessary_picks(struct repository *r,
>  		MOVE_ARRAY(todo_list->items, todo_list->items + i, todo_list->nr - i);
>  		todo_list->nr -= i;
>  		todo_list->current = 0;
> +		todo_list->done_nr += i;
>  
>  		if (is_fixup(peek_command(todo_list, 0)))
>  			record_in_rewritten(base_oid, peek_command(todo_list, 0));