Re: [GSoC][PATCH v2 6/7] rebase -i: rewrite setup_reflog_action() in C

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 6, 2018, 12:58 UTC
Message-ID: <nycvar.QRO.7.76.6.1807061457250.75@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqq8t6soy2f.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Tue, 3 Jul 2018, Junio C Hamano wrote:

Show 25 quoted lines

> Alban Gruin <alban.gruin@gmail.com> writes:
> 
> > +static int run_git_checkout(struct replay_opts *opts, const char *commit,
> > +				int verbose, const char *action)
> > +{
> > +	struct child_process cmd = CHILD_PROCESS_INIT;
> > +
> > +	cmd.git_cmd = 1;
> > +
> > +	argv_array_push(&cmd.args, "checkout");
> > +	argv_array_push(&cmd.args, commit);
> > +	argv_array_pushf(&cmd.env_array, GIT_REFLOG_ACTION "=%s", action);
> > +
> > +	if (verbose)
> > +		return run_command(&cmd);
> > +	return run_command_silent_on_success(&cmd);
> 
> I thought we made this into
> 
> 	if verbose
> 		return run_command
> 	else
> 		return run_command_silently
> 
> to help readers in the previous round already.

FWIW we had quite a couple of reviews in the recent years which pointed out "unnecessary else" after returning or die()ing. Maybe we should make up our minds, and set a consistent rule to follow.

Ciao, Dscho