Re: [PATCH v2 0/6] Unconvolutize push.default=simple

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 30, 2021, 16:22 UTC
Message-ID: <60b3bbd48b00b_b937208d7@natae.notmuch>
In-Reply-To: <87a6oc318s.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> 
> On Sat, May 29 2021, Felipe Contreras wrote:
> 
> > Tired of jumping through hoops trying to understand what the "simple"
> > mode does, I decided to reorganize it up for good so it's crystal
> > clear.
> 
> I'd find the end-state even more readable if this "triangular" wasn't
> passed as a parameter but we just did the top-level dispatch based on
> that, i.e. "simple" is really internally SIMPLE_SIMPLE and
> SIMPLE_TRIANGULAR, why not dispatch on that? Our internal enum doesn't
> need to 1=1 map to the config setting.

Yes, but where are you going to make sure you are in SIMPLE, and not in CURRENT? Surely you are not suggesting to modify git_default_push_config(), which is pretty straightforward. So we would need yet another enum.

I don't think there's a need for that, as you can see on the patch series on top of this, there's not even a need to dispatch anything.

> Part of that's that I prefer reading code in the current "master" which
> is if -> die -> most of the rest of function, v.s. your end-state of if
> -> stuff -> else -> most of the function being indented to that "else".

I prefer that as well, but unfortunately in this case we need to do something after the checks to die(). I could add a goto, but for just one conditional seems like it's not worth it.

Either way this is a temporary thing, because in the next patch series the code becomes pretty quickly:

  if (!triangular && strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
          die_push_simple(branch, remote);

And then:

  if (triangular)
    break;
  if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
    die_push_simple(branch, remote);

Cheers.

-- 
Felipe Contreras