Thread (2 messages) flat view 2 messages, 2 authors, 2021-12-03

Re: [RFC PATCH 1/1] push: make '-u' have default arguments

From: Abhradeep Chakraborty <hidden>
Date: 2021-12-03 08:18:00

Junio C Hamano [off-list ref] wrote:
Taking all together, something like

   "git push -u" (set-upstream) requires where to push to and what
   to push.  Often people push only the current branch to update
   the branch of the same name at the 'origin' repository.  For
   them, it would be convenient if "git push -u" without repository
   or refspec defaulted to push to the branch of the same name at
   the remote repository that is used by default.
Thanks for the guidance. Improving the cover-letter and commit message
now. :)
Do not invent an undefined word "short name".  The name of the
'main' branch is 'main', and it is not a short name.  When people
encounter multi-level names, like ac/push-u-default, use of an
undefined word "short name" will mislead readers that you meant
the leaf level, 'push-u-default', but I do not think that is what
you meant (this is not the only instance of "short name" in this
submission; all need to be fixed).
Sorry for that, I was referring 'branch->name' as 'short name' (and
'branch->refname' as the 'long name' :| ). Will fix it.
One thing that bothers me is that unlike your assumption, not
everybody uses push.default set to simple or upstream.  I am not
convinced that the "git push -u" that defaults to do the 'current'
push with TRANSPORT_PUSH_SET_UPSTREAM for them is an improvement
for them.
May be you're right. It may not be an improvement for all. But I
think they also would be happy seeing this 'default' case of 
'set-upstream'.
If the new feature does not kick in for them, that should
be explained in the proposed log message when you sell the patch to
reviewers and documented for the users.
Ok.
This makes it sound as if the push will only affect the current
branch even for folks who use the matching push.  As I said, I do
not know if that is desirable.
Yeah, this only affects the current branch. In that case they may
not use 'git push -u'. As you said previously, this would not be
an improvement in this case ( not a deterioration also).
This does look like it breaks unless the user is a novice without
custom configuration.  For example, if the current branch has a
configuration to integrate with a branch at the default remote of a
different name already, this (1) clobbers the tip of a wrong branch
by pushing to it, and (2) overrites the upstream configuration.  If
the user uses push.default set to 'current' or 'simple', this would
be OK, but for all other users, I doubt this would be an improvement.
I don't think so. When an user use '--set-upstream' in push command,
it means he/she want to set the upstream to a different branch (if
the specified <repository> and <refspec> are not same as the current
one) rather than the current upstream branch (if exists). So, I think
it would be safe for '--set-upstream' to have default values. Isn't it?
If you are fearing for those two points you specified, should we add
a safety permission before proceeding? e.g. like this -
 
     this would change the upstream branch "%s" to "%s" for this local branch
     would you like to continue? [y]es [n]no
We may want to future-proof by checking the current tracking info
(or lack of it) before doing "git push -u" here?  You cannot control
what other developers would do in the future to tests before this
one.
Oh yeah, you're right. Will surely add it.
Various settings of push.default is probably a good place to start and
with or without existing upstream info already set up.
Thanks for the suggestion, I will add more tests to address these things.
Thanks for working on this topic.  I suspect that the implementation
and design covers too broadly to hurt some users while helping
others, and needs tightening up to fix that, but I think the users
appreciate the part that helps some users ;-)
Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help