diff mbox series

sequencer: remove unreachable exit condition in pick_commits()

Message ID 20230903151132.739151-1-oswald.buddenhagen@gmx.de (mailing list archive)
State Superseded
Headers show
Series sequencer: remove unreachable exit condition in pick_commits() | expand

Commit Message

Oswald Buddenhagen Sept. 3, 2023, 3:11 p.m. UTC
This was introduced by 56dc3ab04 ("sequencer (rebase -i): implement the
'edit' command", 2017-01-02), and was pointless from the get-go, as the
command causes an early return.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

---
this remains valid after phillip's pending series, through it becomes
marginally harder to prove (c.f. "sequencer: factor out part of
pick_commits()").

Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Phillip Wood <phillip.wood123@gmail.com>
---
 sequencer.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Phillip Wood Sept. 12, 2023, 10:18 a.m. UTC | #1
Hi Oswald

On 03/09/2023 16:11, Oswald Buddenhagen wrote:
> This was introduced by 56dc3ab04 ("sequencer (rebase -i): implement the
> 'edit' command", 2017-01-02), and was pointless from the get-go, as the
> command causes an early return.

While I agree this code is unreachable, just because it was unused when 
it was added does not mean it is unused now. It would be helpful for the 
commit message to explain that there are no "break" or "goto" statements 
in the loop body and therefore this code is only reachable when the loop 
terminates because todo_list->current == todo_list->nr

Best Wishes

Phillip

> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> 
> ---
> this remains valid after phillip's pending series, through it becomes
> marginally harder to prove (c.f. "sequencer: factor out part of
> pick_commits()").
> 
> Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> ---
>   sequencer.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index a66dcf8ab2..99e9c520ca 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,
>   		struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
>   		struct stat st;
>   
> -		/* Stopped in the middle, as planned? */
> -		if (todo_list->current < todo_list->nr)
> -			return 0;
> -
>   		if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
>   				starts_with(head_ref.buf, "refs/")) {
>   			const char *msg;
diff mbox series

Patch

diff --git a/sequencer.c b/sequencer.c
index a66dcf8ab2..99e9c520ca 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4832,10 +4832,6 @@  static int pick_commits(struct repository *r,
 		struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
 		struct stat st;
 
-		/* Stopped in the middle, as planned? */
-		if (todo_list->current < todo_list->nr)
-			return 0;
-
 		if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
 				starts_with(head_ref.buf, "refs/")) {
 			const char *msg;