Re: [PATCH 04/11] push: simplify setup_push_simple()

From: Elijah Newren <newren@gmail.com>
Date: May 28, 2021, 20:57 UTC
Message-ID: <CABPp-BG=496bZ78ToKK0vwmmtWDDC4qbCZfXbL+odzecy8U7vw@mail.gmail.com>
In-Reply-To: <20210528201014.2175179-5-felipe.contreras@gmail.com>

On Fri, May 28, 2021 at 1:10 PM Felipe Contreras <felipe.contreras@gmail.com> wrote:

>
> branch->refname can never be different from branch->merge[0]->src.

This statement isn't true without additional qualifications. Perhaps extend your commit message with "...since the 'Additional safety' check dies if they differ." or some other wording that qualifies why it's true at the point of the code in question.

Show 39 quoted lines

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/push.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 9c807ed707..73fe083682 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -225,14 +225,10 @@ static void setup_push_current(struct remote *remote, struct branch *branch)
>
>  static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)
>  {
> -       const char *dst;
> -
>         if (!branch)
>                 die(_(message_detached_head_die), remote->name);
>
> -       if (triangular) {
> -               dst = branch->refname;
> -       } else {
> +       if (!triangular) {
>                 if (!branch->merge_nr || !branch->merge || !branch->remote_name)
>                         die(_("The current branch %s has no upstream branch.\n"
>                             "To push the current branch and set the remote as upstream, use\n"
> @@ -248,10 +244,8 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int
>                 /* Additional safety */
>                 if (strcmp(branch->refname, branch->merge[0]->src))
>                         die_push_simple(branch, remote);
> -
> -               dst = branch->merge[0]->src;
>         }
> -       refspec_appendf(&rs, "%s:%s", branch->refname, dst);
> +       refspec_appendf(&rs, "%s:%s", branch->refname, branch->refname);
>  }
>
>  static int is_workflow_triangular(struct remote *remote)
> --
> 2.32.0.rc0

Simple transformation allowed by the "Additional safety" check; makes sense.