Re: [RFC/PATCH] push: introduce implicit push

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Apr 13, 2013, 04:49 UTC
Message-ID: <CALkWK0=-GcOF17Q-y-Aqj0ThX5pPQFrriDqoJ2qsr=CS+wUNGA@mail.gmail.com>
In-Reply-To: <7v38uvcrjl.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 8 quoted lines

> The primary reason is the confusion factor Jeff mentioned in the
> thread that inspired this patch.  People would realize it is very
> natural to decide where to push to based on what branch is being
> pushed, but only after they think it long and hard enough [*1*].  I
> suspect that it is an equally natural expectation for casual users
> that the destination is chosen based on the current branch, if only
> because that is what they are used to seeing when they say "git
> push" without any argument.

I agree with you largely, but I would still argue that choosing a destination based on the current branch is a historical mistake made by "matching". We don't have to be stuck with this historical mistake, because this is a new syntax: when users read about it in the documentation/ What's New in git.git type email, they will also learn that it chooses the destination based on the refspec.

> Even though I personally am in favor of this "destination is tied to
> what is pushed out", not "destination is chosen based on the current
> branch", I can understand why some people would prefer the latter,
> and why they find it simpler and easier to explain.

Agreed. This is a consequence of not introducing triangular workflows earlier, and getting our users used to distributed workflows. With this patch, users must mandatorily know about remote.pushdefault and branch.<name>.pushremote, if they want to work in multiple-remote scenarios. My argument for that is that multiple-remote workflows have always been a hack until now, and users of that setup will thank us for fixing this.

Show 12 quoted lines

> The second reason is purely on the differences between what the
> above clean-nice explanation says and what the patch actually does.
>
> I think "is-possible-refspec" and "pushremote-get-for-refspec" are
> both way over-engineered, even for people who agree with me and the
> above introduction for this change to favor "destination depends on
> what branch is pushed out".  If is-possible-refspec is replaced with
> a much simpler to understand logic, "Is this a local branch name?",
> possibly combined with "There is no such path on the filesystem" and
> "It's not a defined remote" (iow, reject "git push master:next" and
> anything more complex) [*2*], I suspect it would be a bit more
> sellable.

I don't feel strongly either way, as I just want a simple 'git push master next +pu' to DTRT. I rarely, if ever, specify the :<dst> part of the refpsec. Just so we're clear, we want:

- In git push master, master is verified not to be a path on the
filesystem, not a remote, and finally a local branch.

- In git push master:next, master:next is interpreted as a destination.

- In git push master next:pu, master is verified as usual, and next:pu
is pushed to the remote specified by next.  My patch currently does
this (checks that <src> and <dst> are branches).

- In git push master next:refs/tags/v3.1 and git push master
v3.1:refs/heads/next, master is verified as usual and the refspec is
pushed to the remote specified by remote.pushdefault, falling back to
origin (since the <dst> is not a branch).  My patch currently pushes
it to the current branch's configuration, and I've already marked it
as a TODO.