Ævar Arnfjörð Bjarmason wrote: > > 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