| Message ID | 20210529071115.1908310-2-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Unconvolutize push.default=simple | expand |
Felipe Contreras <felipe.contreras@gmail.com> 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 <newren@gmail.com> 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 <felipe.contreras@gmail.com> > --- > 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:
Junio C Hamano wrote: > Felipe Contreras <felipe.contreras@gmail.com> 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". Those appear to be the intransitive verb definitions of Merriam-Webster [1], there's an object in the sentence (code), so it's the transitive ones that are applicable: * to enclose or protect with or as if with a dense row of shrubs or low trees: ENCIRCLE * to confine so as to prevent freedom of movement or action Some synonyms: block, border, cage, confine, coop, corral, edge, fence, restrict, ring, surround. I think it fits. > > Reviewed-by: Elijah Newren <newren@gmail.com> > > 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). But he did review the entire series. So I think it's safe to say he reviewed this patch. > 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 ;-) All right, I'll keep in mind for the next round. Cheers. [1] https://www.merriam-webster.com/dictionary/hedge
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: