Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jun 20, 2013, 21:22 UTC
Message-ID: <CALkWK0mTthYyHRkxqK2Z3uP3uUmtQADyZt7CERY_40iQSQyzAA@mail.gmail.com>
In-Reply-To: <7vd2rgtwl3.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 10 quoted lines

>> They're not the same thing.  It is very much intentional and intended:
>> the safety net is not to "ensure that the push and pull are
>> symmetrical" (i.e. among other things, error out if
>> branch.$branch.merge is unset), but rather "ensure that the push and
>> pull are never asymmetrical".
>
>     not to "ensure that the push and pull are symmetrical"
>     rather "ensure that the push and pull are never asymmetrical".
>
> They still talk the same thing to me.  What am I missing?

Never mind the wording then. I am proposing preventing asymmetry by explicitly disallowing a push when $branch is different from branch.$branch.merge, instead of shutting down immediately when branch.$branch.merge is unset.

Show 7 quoted lines

>> Now I'd like to question what you are labelling as "safety".  What is
>> the consequence of erroring out when branch.$branch.merge is unset
>> when pushing using `upstream`?
>
> Nothing noteworthy.
>
> I wasn't suggesting to change what `upstream` does at all.

No, but I did. I just argued for a sane default for branch.$branch.merge (the part you snipped out).

> The conclusion is that using push.default=`upstream` is meaningless
> when you are using a triangular workflow.

Yes, and I agreed with that.

> If you are using a
> centralized workflow, and if a branch does not have branch.*.merge
> configured, we do not know to which branch you are pushing it back,
> so we error out.

And I said: have a sane default.

> What I am changing from the patch you posted with the "how about
> this on top" patch back to the current behaviour is what 'simple'
> does for centralized workflow.

Yes, I know.  I read the patch.

> When you are doing a centralized workflow, 'simple' was defined

Again, I'm well aware what it _was_ defined as. Was it not clear that I argued for a change from the very first email where I posted the patch and changed a test? Do you have a counter-argument, or is it simply that we must respect its historical meaning?

Show 7 quoted lines

> Now, no existing series has casted in stone the best behaviour for
> `simple` in a triangular workflow.  With this series (and also with
> my fixup patch I sent last night), it is defined to act as `current`,
> but it may need a bit more safety to help new users avoid pushing
> branches they did not intend to (perhaps pushing out `current` only
> when the branch with the same name already exists at the destination?
> I dunno).

I see no reason to plan safety features in advance, especially since we have neither branch.$branch.push nor @{push} yet.