Subject: Re: [PATCH v3] builtin/revert.c: refactor using an enum for cmd-action

From: Junio C Hamano <[email-masked]>
Date: Thu, 11 Jan 2024 16:40:25 -0800

Michael Lohmann <[email-masked]> writes:

> -	if (cmd == 'c')
> +	case ACTION_CONTINUE:
>  		return sequencer_continue(the_repository, opts);
> -	if (cmd == 'a')
> +	case ACTION_ABORT:
>  		return sequencer_rollback(the_repository, opts);
> -	if (cmd == 's')
> +	case ACTION_SKIP:
>  		return sequencer_skip(the_repository, opts);
> -	return sequencer_pick_revisions(the_repository, opts);
> +	case ACTION_START:
> +		return sequencer_pick_revisions(the_repository, opts);
> +	}
>  }

This change broke the build when merged to 'seen' like so ...

builtin/revert.c: In function 'run_sequencer':
builtin/revert.c:242:1: error: control reaches end of non-void function [-Werror=return-typ ]
  242 | }
      | ^
 
... so I'm discarding
it out of my tree and redoing today's integration cycle.

Different compilers are smart in different ways, and we shouldn't
overly rely on the fact that some compilers may be happy by seeing a
switch that has all the enum values and notice that one of the return
will be triggered in its case arms.