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

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jun 20, 2013, 10:09 UTC
Message-ID: <CALkWK0=v25wC1r8ScUkKDhFjctZCDLJtpDx2g2avyYgJVmZCWg@mail.gmail.com>
In-Reply-To: <7vk3lpwkt6.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 6 quoted lines

>> Decouple `simple` from `upstream` completely, and change it to mean
>> `current` with a safety feature: a `push` and `pull` should not be
>> asymmetrical in the special case of central workflows.
>
> Double negation confused my parser.  'push' and 'pull' should be
> kept symmetrical in central workflows?

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".

Show 13 quoted lines

>> Without any configuration the current branch is pushed out, which
>> loosens the safety we implemented in the current 'safer upstream'.
>>
>> I am not convinced this is a good change.  I am not convinced this is
>> a bad change, either, yet, but this loosening smells bad.
>
> Provided that we would want to keep the "Push the current one to the
> same name but you have to have it set up as your integration source"
> safety for central workflow (which I am starting to think we
> should), we would want something like this on top of your entire
> series, I think.  The behaviour change can be seen in the revert of
> one test you made to the test that expects "simple" to fail due to
> the safety.

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`? For me, it only means additional inconvenience: any new branches I create can't be pushed out without explicitly setting branch.$branch.merge to an "invalid" value. What is invalid about it? The fact that it doesn't exist, @{u} still doesn't resolve, and git branch -u doesn't work. Hell, even git push -u doesn't work! So, what is this huge "safety" that can justify inconveniencing me like this? By making sure that branch.$branch.merge is set, my prompt responds immediately to divergence, and this is awesome. Predictably, I use git push -u when I push out a new branch with `current`. So, unless you have a damn good reason to inconvenience me in the name of safety, branch.$branch.merge should default to refs/heads/$branch, unless set explicitly.

I didn't want to contaminate this series with an unrelated improvement to `upstream`, which is why you don't see the change here: it is orthogonal to designing a good `simple`, and I only brought it up to question the "safety" you're carrying over to `simple`; what obligation does `simple` have to carry over this "feature"? I've made it clear that I want a clean break from `upstream`, and I find your proposal is very inelegant: `simple` has two modes of operation; when branch.$branch.remote is equal to $pushremote, branch.$branch.merge must be set and equal to $branch (the `upstream` mode); when branch.$branch.remote is unequal to $pushremote, don't care about whether branch.$branch.merge is set (the `current` mode). My proposal is much smoother than this "modal" operation, no?