Re: [GSoC][PATCH v5 2/3] rebase -i: rewrite setup_reflog_action() in C

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 29, 2018, 16:50 UTC
Message-ID: <xmqqvaa1v8op.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180629151435.31868-3-alban.gruin@gmail.com>

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

Show 18 quoted lines

> +	git rebase--helper --prepare-branch "$switch_to" ${verbose:+--verbose}
>  	init_basic_state
>  
>  	init_revisions_and_shortrevisions
> diff --git a/sequencer.c b/sequencer.c
> index d9545b366..dd0cf3cb5 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3134,6 +3134,37 @@ static const char *reflog_message(struct replay_opts *opts,
>  	return buf.buf;
>  }
> ...
> +int prepare_branch_to_be_rebased(struct replay_opts *opts, const char *commit,
> +				 int verbose)
> +{
> +	const char *action;
> +
> +	if (commit && *commit) {

The sloppyness of this callee is forced by the sloppy caller, which does not check if "$switch_to" has any value before calling "prepare branch to be rebased" function. A less sloppy caller would check to see if we have the optional "before doing anything, check out this branch, as it is the branch we are trying to rebase" argument, and refrain from calling this function, so there is no need for this "if commit is given and is not an empty string" check.

And hopefully, when GSoC is over, the caller, that is still in shell at this step, would also be rewritten in C and by that time the caller will become less sloppy, removing the need for this check.

Hence, I think the reason why the check is still here, and our desire to eventually remove the check, should be documented with an in-code comment around here (the usual "NEEDSWORK:" comment is fine).