Re: [PATCH 6/6] push: honor branch.*.push

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jun 24, 2013, 17:13 UTC
Message-ID: <CALkWK0nv7On34g-zE5uHAH69TT-KcEt8z-LxhveNp1b4b+NHsg@mail.gmail.com>
In-Reply-To: <7vbo6vbgbs.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 10 quoted lines

> If you do not set anywhere (like branch.master.pushremote or
> remote.pushdefault) to push to "ram", and if you did not say "git
> push ram" but just said "git push", we will not push to "ram"
> (otherwise it is broken).  So if the push is going to "ram", the
> user must have asked us to push there, either via the command line,
> or these explicit configuration variables.  And why do you need to
> differenciate between the command line "ram" and configured "ram"?
> After all, isn't the configuration merely a typesaver?  If you know
> often push to "ram", you configure to push there.  If you don't, you
> don't.

Yes, you're talking about configuration variables in general.  If the user says

  $ git status

expecting long-form output, but gets short-form output, the world hasn't ended. But if the user does

  $ git push -f

expecting the push to go to one place, when it actually goes to another place, she would have killed a few co-workers.

I'm not saying that we need to differentiate between configuration variables and CLI options; what I _am_ saying is that we need to think twice about moving a CLI option to a configuration variable, precisely because we do not differentiate between the two cases.

As I have said earlier, my philosophy to prevent disasters does not involve erroring-out or denying for the most part; rather it focuses on giving the user immediate feedback like:

  $ git push -f
  # pushing master:next to ram (press ^C to abort)

Another form of feedback is the refspec @{p[ush]}, which I'm struggling to complete.

> The push.default is meant to be in effect only when there is no
> other stronger clue from the user for what to update (e.g. command
> line refspec, remote.*.push).

Yes, I know what it does currently. With your patch, the list becomes "push.default will be in effect only when there is no CLI refspec, remote.<name>.push or branch.<name>.push in effect". I'm wondering if there's a more elegant way to do it.

Show 6 quoted lines

> Of course, that requires, as you earlier pointed out, that the logic
> to read from branch.*.push need to be moved out of the push.default
> logic (in the weatherbaloon patch) and hosted to do_push() where it
> checks if there is remote->pushrefspec[].  If branch.*.push wants to
> defeat remote.*.push, then it should be checked before deciding to
> use that configured remote.*.push.

I suspect there's a better way to do this.

> As I already said, I personally do not know what branch.*.push buys
> us, and will happy to drop 6/6.  It is merely a weatherbaloon patch.

I hope we come out with a better version soon.