Subject: Re: [PATCH v2 1/6] push: hedge code of default=simple

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

Felipe Contreras <[email-masked]> writes:

> `simple` is the most important mode so move the relevant code to its own
> function to make it easier to see what it's doing.

The change and the above description makes sense to me.

I didn't, and still don't, understand the use of verb "to hedge" in
the title, though.  Apparently it isn't "to evade the risk of
commitment", "to protect oneself finantially", and of course not "to
plant, form, or trim a hedge".

> Reviewed-by: Elijah Newren <[email-masked]>

I trust Elijah would complain and/or clarify if this footer is
inappropriate (I didn't see an explicit "You can have my
Reviewed-by", but only "this looks good to me", and didn't know
what he meant).

I do like the change of the phrasing from triangular to same-remote
at the end of the extended series, by the way.  It makes the code
simpler to read and much easier to reason about, and it would be
nice to have it even before this step ;-)

> Signed-off-by: Felipe Contreras <[email-masked]>
> ---
>  builtin/push.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 194967ed79..7045e4ef0c 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -223,6 +223,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)
>  	refspec_appendf(&rs, "%s:%s", branch->refname, branch->refname);
>  }
>  
> +static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)
> +{
> +	if (triangular)
> +		setup_push_current(remote, branch);
> +	else
> +		setup_push_upstream(remote, branch, triangular, 1);
> +}
> +
>  static int is_workflow_triangular(struct remote *remote)
>  {
>  	struct remote *fetch_remote = remote_get(NULL);
> @@ -242,10 +250,7 @@ static void setup_default_push_refspecs(struct remote *remote)
>  
>  	case PUSH_DEFAULT_UNSPECIFIED:
>  	case PUSH_DEFAULT_SIMPLE:
> -		if (triangular)
> -			setup_push_current(remote, branch);
> -		else
> -			setup_push_upstream(remote, branch, triangular, 1);
> +		setup_push_simple(remote, branch, triangular);
>  		break;
>  
>  	case PUSH_DEFAULT_UPSTREAM: