Subject: Re: [PATCH 08/15] push: get rid of all the setup_push_* functions

From: Junio C Hamano <[email-masked]>
Date: Mon, 31 May 2021 15:44:55 +0900

Felipe Contreras <[email-masked]> writes:

> Their code is much simpler now and can move into the parent function.
>
> Signed-off-by: Felipe Contreras <[email-masked]>
> ---
>  builtin/push.c | 38 +++++++++++---------------------------
>  1 file changed, 11 insertions(+), 27 deletions(-)

Finally, this resolves the risk of deliberately duplicated code of
diverging, which was made in the earlier series.  Without seeing
this step, it has been dubious if the earlier change was a good one,
but with this step, it is a very clear win.

Nicely done.

> diff --git a/builtin/push.c b/builtin/push.c
> index 21968abf6e..dbb4f78e61 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -202,30 +202,6 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na
>  	return branch->merge[0]->src;
>  }
>  
> -static const char *setup_push_upstream(struct remote *remote, struct branch *branch,
> -	int triangular)
> -{
> -	if (triangular)
> -		die(_("You are pushing to remote '%s', which is not the upstream of\n"
> -		      "your current branch '%s', without telling me what to push\n"
> -		      "to update which remote branch."),
> -		    remote->name, branch->name);
> -	return get_upstream_ref(branch, remote->name);
> -}
> -
> -static const char *setup_push_current(struct remote *remote, struct branch *branch)
> -{
> -	return branch->refname;
> -}
> -
> -static const char *setup_push_simple(struct remote *remote, struct branch *branch, int triangular)
> -{
> -	if (!triangular)
> -		if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
> -			die_push_simple(branch, remote);
> -	return branch->refname;
> -}
> -
>  static int is_workflow_triangular(struct remote *remote)
>  {
>  	struct remote *fetch_remote = remote_get(NULL);
> @@ -258,15 +234,23 @@ static void setup_default_push_refspecs(struct remote *remote)
>  	default:
>  	case PUSH_DEFAULT_UNSPECIFIED:
>  	case PUSH_DEFAULT_SIMPLE:
> -		dst = setup_push_simple(remote, branch, triangular);
> +		if (!triangular)
> +			if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
> +				die_push_simple(branch, remote);
> +		dst = branch->refname;
>  		break;
>  
>  	case PUSH_DEFAULT_UPSTREAM:
> -		dst = setup_push_upstream(remote, branch, triangular);
> +		if (triangular)
> +			die(_("You are pushing to remote '%s', which is not the upstream of\n"
> +			      "your current branch '%s', without telling me what to push\n"
> +			      "to update which remote branch."),
> +			    remote->name, branch->name);
> +		dst = get_upstream_ref(branch, remote->name);
>  		break;
>  
>  	case PUSH_DEFAULT_CURRENT:
> -		dst = setup_push_current(remote, branch);
> +		dst = branch->refname;
>  		break;
>  	}