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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux