[PATCH] config doc: rewrite push.default section

[PATCH] config doc: rewrite push.default section

[Ramkumar Ramachandra]

Design by Junio.

By detaching descriptions from the implementation, we're only confusing
users.  I've chosen to use the term "central workflow" to make the
descriptions terse and readable, although I've stayed way from
"triangular workflow" (referred to as non-central workflow).

Yes, I hate writing documentation but I have no choice if I want to
update the implementations to do something sane in triangular workflows.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 I'll send in the implementation once we can agree that this is what
 we want.

 Documentation/config.txt | 51 ++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7fd4035..30350a3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1832,33 +1832,32 @@ push.default::
 	line. Possible values are:
 +
 --
-* `nothing` - do not push anything.
-* `matching` - push all branches having the same name in both ends.
-  This is for those who prepare all the branches into a publishable
-  shape and then push them out with a single command.  It is not
-  appropriate for pushing into a repository shared by multiple users,
-  since locally stalled branches will attempt a non-fast forward push
-  if other users updated the branch.
-  +
-  This is currently the default, but Git 2.0 will change the default
-  to `simple`.
-* `upstream` - push the current branch to its upstream branch
-  (`tracking` is a deprecated synonym for this).
-  With this, `git push` will update the same remote ref as the one which
-  is merged by `git pull`, making `push` and `pull` symmetrical.
-  See "branch.<name>.merge" for how to configure the upstream branch.
-* `simple` - like `upstream`, but refuses to push if the upstream
-  branch's name is different from the local one. This is the safest
-  option and is well-suited for beginners. It will become the default
-  in Git 2.0.
-* `current` - push the current branch to a branch of the same name.
+* `nothing` - error out unless a refspec is explicitly given.
+
+* `current` - push the refspec "$HEAD".  HEAD is resolved early to a
+  branch name (referred to as $HEAD).  In other words, push the
+  current branch to update a branch with the same name on the pushing
+  side.
+
+* `upstream` - push the refspec "$HEAD:branch.$HEAD.merge", and error
+  out if the push destination is not the same as branch.$HEAD.remote.
+  The name "upstream" refers to the revision "@{u[pstream]}" in
+  linkgit:gitrevisions[7].  It is useful in central workflows, to make
+  the `push` symmetrical to `pull`.
+
+* `simple` - in central workflows, behaves like `upstream`, except
+  that it errors out unless branch.$HEAD.merge is equal to $HEAD.  In
+  non-central workflows, behaves like `current`.  It will become the
+  default in Git 2.0.
+
+* `matching` - push the refspec ":".  In other words, push all
+  branches having the same name in both ends, even if it means
+  non-fast-forward updates.  This is for those who prepare all the
+  branches into a publishable shape and then push them out with a
+  single command.  Dangerous, and inappropriate unless you are the
+  only person updating your push destination.  This is currently the
+  default, but Git 2.0 will change the default to `simple`.
 --
-+
-The `simple`, `current` and `upstream` modes are for those who want to
-push out a single branch after finishing work, even when the other
-branches are not yet ready to be pushed out. If you are working with
-other people to push into the same shared repository, you would want
-to use one of these.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
-- 
1.8.3.1.443.g4fd77b9

Re: [PATCH] config doc: rewrite push.default section

[Philip Oakley]

From: "Ramkumar Ramachandra" <artagnon@gmail.com>
Sent: Sunday, June 16, 2013 11:06 AM
> Design by Junio.
>
> By detaching descriptions from the implementation, we're only 
> confusing
> users.  I've chosen to use the term "central workflow" to make the
> descriptions terse and readable, although I've stayed way from
> "triangular workflow" (referred to as non-central workflow).

A sentence, in the Documentation/config.txt, is needed to clarify the 
Central workflow and any distinction with the  non-central workflow(s). 
We cannot assume the new reader has the same world view of that concept 
(they may be thinking it means we do a centralised VCS, not a DVCS with 
a chosen central primary repo - assuming I have understood it 
correctly).

It took a while to bottom out the issues, so it is worth summarising the 
key point(s) in the documentation to avoid having to repeat the 
disussions ;-)

>
> Yes, I hate writing documentation but I have no choice if I want to
> update the implementations to do something sane in triangular 
> workflows.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> I'll send in the implementation once we can agree that this is what
> we want.
>
> Documentation/config.txt | 51 
> ++++++++++++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 7fd4035..30350a3 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1832,33 +1832,32 @@ push.default::
>  line. Possible values are:
> +
> --
> -* `nothing` - do not push anything.
> -* `matching` - push all branches having the same name in both ends.
> -  This is for those who prepare all the branches into a publishable
> -  shape and then push them out with a single command.  It is not
> -  appropriate for pushing into a repository shared by multiple users,
> -  since locally stalled branches will attempt a non-fast forward push
> -  if other users updated the branch.
> -  +
> -  This is currently the default, but Git 2.0 will change the default
> -  to `simple`.
> -* `upstream` - push the current branch to its upstream branch
> -  (`tracking` is a deprecated synonym for this).
> -  With this, `git push` will update the same remote ref as the one 
> which
> -  is merged by `git pull`, making `push` and `pull` symmetrical.
> -  See "branch.<name>.merge" for how to configure the upstream branch.
> -* `simple` - like `upstream`, but refuses to push if the upstream
> -  branch's name is different from the local one. This is the safest
> -  option and is well-suited for beginners. It will become the default
> -  in Git 2.0.
> -* `current` - push the current branch to a branch of the same name.
> +* `nothing` - error out unless a refspec is explicitly given.
> +
> +* `current` - push the refspec "$HEAD".  HEAD is resolved early to a
> +  branch name (referred to as $HEAD).  In other words, push the

s/In other words,/That is,/
'In other words' often indicates poor wording, while here the extra 
words explicitly explain the effect.

> +  current branch to update a branch with the same name on the pushing
> +  side.

s/pushing side/push destination/ for consistency with upstream wording 
used below.

> +
> +* `upstream` - push the refspec "$HEAD:branch.$HEAD.merge", and error
> +  out if the push destination is not the same as branch.$HEAD.remote.
> +  The name "upstream" refers to the revision "@{u[pstream]}" in
> +  linkgit:gitrevisions[7].  It is useful in central workflows, to 
> make
> +  the `push` symmetrical to `pull`.
> +
> +* `simple` - in central workflows, behaves like `upstream`, except
> +  that it errors out unless branch.$HEAD.merge is equal to $HEAD.  In
> +  non-central workflows, behaves like `current`.  It will become the
> +  default in Git 2.0.
> +
> +* `matching` - push the refspec ":".  In other words, push all
> +  branches having the same name in both ends, even if it means
> +  non-fast-forward updates.  This is for those who prepare all the
> +  branches into a publishable shape and then push them out with a
> +  single command.  Dangerous, and inappropriate unless you are the

"Dangerous and innappropriate" (which it maybe for some) is too 
judgemental.
Perhaps turn it around to a positive (unless -> only if).

"Useful if you are the.."

> +  only person updating your push destination.  This is currently the
> +  default, but Git 2.0 will change the default to `simple`.
> --
> -+
> -The `simple`, `current` and `upstream` modes are for those who want 
> to
> -push out a single branch after finishing work, even when the other
> -branches are not yet ready to be pushed out. If you are working with
> -other people to push into the same shared repository, you would want
> -to use one of these.
>
> rebase.stat::
>  Whether to show a diffstat of what changed upstream since the last
> -- 
> 1.8.3.1.443.g4fd77b9
>
> --
regards
Philip

Re: [PATCH] config doc: rewrite push.default section

[Ramkumar Ramachandra]

Philip Oakley wrote:
> A sentence, in the Documentation/config.txt, is needed to clarify the
> Central workflow and any distinction with the  non-central workflow(s). We
> cannot assume the new reader has the same world view of that concept (they
> may be thinking it means we do a centralised VCS, not a DVCS with a chosen
> central primary repo - assuming I have understood it correctly).
>
> It took a while to bottom out the issues, so it is worth summarising the key
> point(s) in the documentation to avoid having to repeat the disussions ;-)

I'm not sure where to put it: Documentation/gitworkflows.txt perhaps?

> [...]

Agreed with the rest.  Thanks.

Re: [PATCH] config doc: rewrite push.default section

[Matthieu Moy]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> +* `current` - push the refspec "$HEAD".  HEAD is resolved early to a
> +  branch name (referred to as $HEAD).  In other words, push the
> +  current branch to update a branch with the same name on the pushing
> +  side.

I'd put it the other way around: the intuitive explanation first, and
the technical one after. For people not totally familiar with Git, the
first part does not make much sense (and when I learn a new tool, I
really don't like when the doc assumes I already know too much about
it).

Also, this $HEAD Vs HEAD doesn't seem very clear to me. I don't have a
really good proposal for a better wording, but maybe replacing $HEAD
with $branch would make a bit more sense, as having $HEAD != HEAD is
weird.

> +* `simple` - in central workflows, behaves like `upstream`, except
> +  that it errors out unless branch.$HEAD.merge is equal to $HEAD.

I'd reverse the sentense too. In your wording, I get the feeling that
erroring out is the normal flow, and pushing is the exception
("unless").

"... except that it errors out if branch.$HEAD.merge is not equal to
$HEAD." ?

> +  single command.  Dangerous, and inappropriate unless you are the
> +  only person updating your push destination.

Here also, I'd have said "Dangerous, and inappropriate if you are
not ...".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config doc: rewrite push.default section

[Ramkumar Ramachandra]

Matthieu Moy wrote:
> I'd put it the other way around: the intuitive explanation first, and
> the technical one after. For people not totally familiar with Git, the
> first part does not make much sense (and when I learn a new tool, I
> really don't like when the doc assumes I already know too much about
> it).

Good.

> Also, this $HEAD Vs HEAD doesn't seem very clear to me. I don't have a
> really good proposal for a better wording, but maybe replacing $HEAD
> with $branch would make a bit more sense, as having $HEAD != HEAD is
> weird.

Good.

>> +* `simple` - in central workflows, behaves like `upstream`, except
>> +  that it errors out unless branch.$HEAD.merge is equal to $HEAD.
>
> "... except that it errors out if branch.$HEAD.merge is not equal to
> $HEAD." ?

Good.

>> +  single command.  Dangerous, and inappropriate unless you are the
>> +  only person updating your push destination.
>
> Here also, I'd have said "Dangerous, and inappropriate if you are
> not ...".

I might have overplayed the danger a bit, as Junio points out.  I'll
have a look at your --force documentation patch.

Re: [PATCH] config doc: rewrite push.default section

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Design by Junio.

Not necessary.  The conclusion of discussion is a result of
collaboration.

Thanks for writing it down.  It is a good start, but I agree with
reviews by Philip Oakley and Matthieu Moy we already saw.  

 - To understand "if central, works as upstream, otherwise works as
   current", the readers need to know if their workflow is 'central'
   or not, so we need to say how that is decided upfront (probably
   immediately before "Possible values are:" in the introductory
   paragraph for push.default;

 - For each of these choices, what it means is more important to the
   readers than how the implementation achieves that semantics
   (e.g. "current uses refs/heads/foo:refs/heads/foo when you are on
   foo branch"). I think the ideal is a description of the meaning
   that is clear enough not to require any implementation detail.

I do no think the remainder (snipped) belongs to the log message.

As this is an attempt to _define_ the semantics of what should
happen in triangular workflow, I think it would be healthy to wait
for a week or so in order for others (not just two of us) can see if
we have defined a sane semantics we would want to go forward with.
I am reasonably sure 'current' would be the best default for
triangular (and that is why I suggested 'simple' to fall back to
it), but I do not think others had a chance to see what design our
discussion settled, think if that makes sense, and think of a better
alternative.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 7fd4035..30350a3 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1832,33 +1832,32 @@ push.default::
>  	line. Possible values are:
>  +
>  --
> -* `nothing` - do not push anything.
> -* `matching` - push all branches having the same name in both ends.
> -  This is for those who prepare all the branches into a publishable
> -  shape and then push them out with a single command.  It is not
> -  appropriate for pushing into a repository shared by multiple users,
> -  since locally stalled branches will attempt a non-fast forward push
> -  if other users updated the branch.
> -  +
> -  This is currently the default, but Git 2.0 will change the default
> -  to `simple`.
> -* `upstream` - push the current branch to its upstream branch
> -  (`tracking` is a deprecated synonym for this).
> -  With this, `git push` will update the same remote ref as the one which
> -  is merged by `git pull`, making `push` and `pull` symmetrical.
> -  See "branch.<name>.merge" for how to configure the upstream branch.
> -* `simple` - like `upstream`, but refuses to push if the upstream
> -  branch's name is different from the local one. This is the safest
> -  option and is well-suited for beginners. It will become the default
> -  in Git 2.0.
> -* `current` - push the current branch to a branch of the same name.

> +* `nothing` - error out unless a refspec is explicitly given.

I do not think 'error out' is the primary effect of this mode.
Wouldn't "do not push anything" be much better?

> +* `current` - push the refspec "$HEAD".  HEAD is resolved early to a
> +  branch name (referred to as $HEAD).  In other words, push the
> +  current branch to update a branch with the same name on the pushing
> +  side.

As already pointed out, dropping everything before and including "In
other words, " would be better.  Also "push the current branch" is
talking about the branch on the pushing side (you, the one who is
running "git push"), and the side that is getting updated is at the
receiving end, not "pushing side".

> +* `upstream` - push the refspec "$HEAD:branch.$HEAD.merge", and error
> +  out if the push destination is not the same as branch.$HEAD.remote.
> +  The name "upstream" refers to the revision "@{u[pstream]}" in
> +  linkgit:gitrevisions[7].  It is useful in central workflows, to make
> +  the `push` symmetrical to `pull`.
> +
> +* `simple` - in central workflows, behaves like `upstream`, except
> +  that it errors out unless branch.$HEAD.merge is equal to $HEAD.  In
> +  non-central workflows, behaves like `current`.  It will become the
> +  default in Git 2.0.

I think listing 'simple' at the end would be easier to read.  The
above already swaps the order to make sure current and upstream are
described before it, which is good.

But I do not see a reason to move 'matching' which was next to
'nothing' here. The 

> +* `matching` - push the refspec ":".  In other words, push all
> +  branches having the same name in both ends, even if it means
> +  non-fast-forward updates.  This is for those who prepare all the
> +  branches into a publishable shape and then push them out with a
> +  single command.  Dangerous, and inappropriate unless you are the
> +  only person updating your push destination.

It was already pointed out that unnecessary negativity needs to be
fixed, but more importantly the above "Dangerous" is not even
correct.

If you are "I am done with this one branch, I want to push it out,
the other branches I have touched but they are not yet in a good
shape to be pushed out" kind of person, it does not matter if you
are the only one who is pushing to the destination.  You would end
up pushing out half-baked ones as well if you used 'matching', which
is not what you want.

> -+
> -The `simple`, `current` and `upstream` modes are for those who want to
> -push out a single branch after finishing work, even when the other
> -branches are not yet ready to be pushed out. If you are working with
> -other people to push into the same shared repository, you would want
> -to use one of these.

And I do not see a reason to drop this part, especially the first
sentence.

Re: [PATCH] config doc: rewrite push.default section

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

>> +* `matching` - push the refspec ":".  In other words, push all
>> +  branches having the same name in both ends, even if it means
>> +  non-fast-forward updates.  This is for those who prepare all the
>> +  branches into a publishable shape and then push them out with a
>> +  single command.  Dangerous, and inappropriate unless you are the
>> +  only person updating your push destination.
>
> It was already pointed out that unnecessary negativity needs to be
> fixed, but more importantly the above "Dangerous" is not even
> correct.

What's really dangerous is the --force flag. A few weeks ago I had to
help a colleague who did a "git push --force" to update his branch, and
he lost data on his co-worker's branches (thanks to "git reflog", it
wasn't an actual data loss, but still pretty bad).

But then the place to warn loudly is the doc for --force. What about
this?

------- 8< ------- 8< ------- 8< ------- 8< ------- 8< ------- 8< 

>From a529588dd8df84e54e5ec267068248cc555373f5 Mon Sep 17 00:00:00 2001
From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Mon, 17 Jun 2013 13:02:39 +0200
Subject: [PATCH] Documentation/git-push.txt: explain better cases where
 --force is dangerous

The behavior of "git push --force" is rather clear when it updates only
one remote ref, but running it when pushing several branches can really
be dangerous. Warn the users a bit more and give them the alternative to
push only one branch.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-push.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 938d1ee..0899a35 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -136,6 +136,13 @@ already exists on the remote side.
        not an ancestor of the local ref used to overwrite it.
        This flag disables the check.  This can cause the
        remote repository to lose commits; use it with care.
+       Note that `--force` applies to all the refs that are pushed,
+       hence using `git push --all --force`, or `git push --force`
+       with `push.default` set to `matching` may override refs other
+       than the current branch (including local refs that are
+       strictly behind their remote counterpart). To force a push to
+       only one branch, use `git push <remote> +<branch>` instead of
+       `--force`.
 
 --repo=<repository>::
        This option is only relevant if no <repository> argument is
-- 
1.8.3.1.495.g13f33cf.dirty


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config doc: rewrite push.default section

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> But then the place to warn loudly is the doc for --force. What about
> this?

Sounds sensible.  I am not sure if "--all" is all that common to be
singled out, though.  "I always push these out" refspecs, like

	[remote "origin"]
        	push = refs/heads/master
        	push = refs/heads/next

share the need for the same cautiousness against --force, and
"applies to all the refs that are pushed" already covers both.

Mentioning 'matching' here is a very good idea, as people may not
realize it is pushing out more than the current branch.

> ------- 8< ------- 8< ------- 8< ------- 8< ------- 8< ------- 8< 
>
> From a529588dd8df84e54e5ec267068248cc555373f5 Mon Sep 17 00:00:00 2001
> From: Matthieu Moy <Matthieu.Moy@imag.fr>
> Date: Mon, 17 Jun 2013 13:02:39 +0200
> Subject: [PATCH] Documentation/git-push.txt: explain better cases where
>  --force is dangerous
>
> The behavior of "git push --force" is rather clear when it updates only
> one remote ref, but running it when pushing several branches can really
> be dangerous. Warn the users a bit more and give them the alternative to
> push only one branch.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  Documentation/git-push.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 938d1ee..0899a35 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -136,6 +136,13 @@ already exists on the remote side.
>         not an ancestor of the local ref used to overwrite it.
>         This flag disables the check.  This can cause the
>         remote repository to lose commits; use it with care.
> +       Note that `--force` applies to all the refs that are pushed,
> +       hence using `git push --all --force`, or `git push --force`
> +       with `push.default` set to `matching` may override refs other
> +       than the current branch (including local refs that are
> +       strictly behind their remote counterpart). To force a push to
> +       only one branch, use `git push <remote> +<branch>` instead of
> +       `--force`.
>  
>  --repo=<repository>::
>         This option is only relevant if no <repository> argument is
> -- 
> 1.8.3.1.495.g13f33cf.dirty

Re: [PATCH] config doc: rewrite push.default section

[Philip Oakley]

From: "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>
Sent: Monday, June 17, 2013 12:09 PM
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> +* `matching` - push the refspec ":".  In other words, push all
>>> +  branches having the same name in both ends, even if it means
>>> +  non-fast-forward updates.  This is for those who prepare all the
>>> +  branches into a publishable shape and then push them out with a
>>> +  single command.  Dangerous, and inappropriate unless you are the
>>> +  only person updating your push destination.
>>
>> It was already pointed out that unnecessary negativity needs to be
>> fixed, but more importantly the above "Dangerous" is not even
>> correct.
>
> What's really dangerous is the --force flag. A few weeks ago I had to
> help a colleague who did a "git push --force" to update his branch, 
> and
> he lost data on his co-worker's branches (thanks to "git reflog", it
> wasn't an actual data loss, but still pretty bad).
>
> But then the place to warn loudly is the doc for --force. What about
> this?
>
> ------- 8< ------- 8< ------- 8< ------- 8< ------- 8< ------- 8<
>
> From a529588dd8df84e54e5ec267068248cc555373f5 Mon Sep 17 00:00:00 2001
> From: Matthieu Moy <Matthieu.Moy@imag.fr>
> Date: Mon, 17 Jun 2013 13:02:39 +0200
> Subject: [PATCH] Documentation/git-push.txt: explain better cases 
> where
> --force is dangerous
>
> The behavior of "git push --force" is rather clear when it updates 
> only
> one remote ref, but running it when pushing several branches can 
> really
> be dangerous. Warn the users a bit more and give them the alternative 
> to
> push only one branch.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Documentation/git-push.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 938d1ee..0899a35 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -136,6 +136,13 @@ already exists on the remote side.
>        not an ancestor of the local ref used to overwrite it.
>        This flag disables the check.  This can cause the
>        remote repository to lose commits; use it with care.
> +       Note that `--force` applies to all the refs that are pushed,
> +       hence using `git push --all --force`, or `git push --force`
> +       with `push.default` set to `matching` may override refs other
> +       than the current branch (including local refs that are
> +       strictly behind their remote counterpart). To force a push to
> +       only one branch, use `git push <remote> +<branch>` instead of
> +       `--force`.

It would be useful to include a real example "e.g. `git push origin 
+master`", or a link to specifying a refspec "see <refspec>... above", 
such that the "+" doesn't get lost in the general text, as push is one 
of the first few commands a new user is likely to be looking up (and 
misunderstanding ;-), so let's make the + obvious

I did notice that the <refspec>... section doesn't actually associate 
the "+" with the force action - Am I misunderstanding this?

>
> --repo=<repository>::
>        This option is only relevant if no <repository> argument is
> -- 
> 1.8.3.1.495.g13f33cf.dirty
>
>
> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2013.0.3345 / Virus Database: 3199/6417 - Release Date: 
> 06/16/13
> 

Re: [PATCH] config doc: rewrite push.default section

[Matthieu Moy]

"Philip Oakley" <philipoakley@iee.org> writes:

>> +       Note that `--force` applies to all the refs that are pushed,
>> +       hence using `git push --all --force`, or `git push --force`
>> +       with `push.default` set to `matching` may override refs other
>> +       than the current branch (including local refs that are
>> +       strictly behind their remote counterpart). To force a push to
>> +       only one branch, use `git push <remote> +<branch>` instead of
>> +       `--force`.
>
> It would be useful to include a real example "e.g. `git push origin
> +master`", or a link to specifying a refspec "see <refspec>... above",
> such that the "+" doesn't get lost in the general text, as push is one
> of the first few commands a new user is likely to be looking up (and
> misunderstanding ;-), so let's make the + obvious

Yes, why not. I'll point to the <refspec> section for detail, and just
give an example here.

> I did notice that the <refspec>... section doesn't actually associate
> the "+" with the force action - Am I misunderstanding this?

It says:

  By having the optional leading `+`, you can tell Git to update the
  <dst> ref even if it is not allowed by default (e.g., it is not a
  fast-forward.)"

I think it's OK.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config doc: rewrite push.default section

[Philip Oakley]

From: "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>
Sent: Monday, June 17, 2013 6:20 PM
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>>> +       Note that `--force` applies to all the refs that are pushed,
>>> +       hence using `git push --all --force`, or `git push --force`
>>> +       with `push.default` set to `matching` may override refs 
>>> other
>>> +       than the current branch (including local refs that are
>>> +       strictly behind their remote counterpart). To force a push 
>>> to
>>> +       only one branch, use `git push <remote> +<branch>` instead 
>>> of
>>> +       `--force`.
>>
>> It would be useful to include a real example "e.g. `git push origin
>> +master`", or a link to specifying a refspec "see <refspec>... 
>> above",
>> such that the "+" doesn't get lost in the general text, as push is 
>> one
>> of the first few commands a new user is likely to be looking up (and
>> misunderstanding ;-), so let's make the + obvious
>
> Yes, why not. I'll point to the <refspec> section for detail, and just
> give an example here.
>
>> I did notice that the <refspec>... section doesn't actually associate
>> the "+" with the force action - Am I misunderstanding this?
>
> It says:
>
>  By having the optional leading `+`, you can tell Git to update the
>  <dst> ref even if it is not allowed by default (e.g., it is not a
>  fast-forward.)"
>
> I think it's OK.

I was more noting that there is zero direct association in the text 
between the --force option, and the "+", and with that, a funny feeling 
that either (a) they had subtle differences I hadn't understood, or (b) 
they were exactly the same and the documenation was being too subtle and 
a cluebat should be applied to the documenation (on the principle I am 
not a unique fool ;-)

>
> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

Maybe...
------- 8< ------- 
From 57d8aaac6b7543919aaf09909c13a180722c0a94 Mon Sep 17 00:00:00 2001
From: Philip Oakley <philipoakley@iee.org>
Date: Mon, 17 Jun 2013 18:47:04 +0100
Subject: [PATCH] git-push doc: +<dst> refspec is --force

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 Documentation/git-push.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index eb2883c..df92b09 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -136,6 +136,8 @@ already exists on the remote side.
  not an ancestor of the local ref used to overwrite it.
  This flag disables the check.  This can cause the
  remote repository to lose commits; use it with care.
+ See also the optional leading `+` <dst> ref specifier in
+ '<refspec>...' above.

 --repo=<repository>::
  This option is only relevant if no <repository> argument is
-- 
1.8.1.msysgit.1

Re: [PATCH] config doc: rewrite push.default section

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> I do no think the remainder (snipped) belongs to the log message.

Oh, it was never intended to be a proper commit message.  I'll write a
proper one when I send it in with the implementation.

>  - To understand "if central, works as upstream, otherwise works as
>    current", the readers need to know if their workflow is 'central'
>    or not, so we need to say how that is decided upfront (probably
>    immediately before "Possible values are:" in the introductory
>    paragraph for push.default;

Good idea.

>  - For each of these choices, what it means is more important to the
>    readers than how the implementation achieves that semantics
>    (e.g. "current uses refs/heads/foo:refs/heads/foo when you are on
>    foo branch"). I think the ideal is a description of the meaning
>    that is clear enough not to require any implementation detail.

I'd definitely like to include the push refspec, to educate users on
how to achieve a one-off matching-push even when push.default is set
to current, for example.

> As this is an attempt to _define_ the semantics of what should
> happen in triangular workflow, I think it would be healthy to wait
> for a week or so in order for others (not just two of us) can see if
> we have defined a sane semantics we would want to go forward with.

Agreed.

> I am reasonably sure 'current' would be the best default for
> triangular (and that is why I suggested 'simple' to fall back to
> it), but I do not think others had a chance to see what design our
> discussion settled, think if that makes sense, and think of a better
> alternative.

You didn't realize one thing, because my wording was terrible:
`simple` is just a "safe" version of `current` :)

* `simple` - behaves exactly like `current`, with one safety feature:
in central workflows, it errors out if branch.$branch.merge is set and
is not equal to $branch (to make sure that the `push` and `pull`
aren't asymmetrical).

In other words, it's `current` with the safety feature of `upstream`.
There's no point in erroring out if no upstream is set, or mentioning
its relationship with `upstream`.

>> +* `nothing` - error out unless a refspec is explicitly given.
>
> I do not think 'error out' is the primary effect of this mode.
> Wouldn't "do not push anything" be much better?

"Do not push anything" is misleading because it implies an exit status
of zero; what is really does is "error out", no?

>> +* `current` - push the refspec "$HEAD".  HEAD is resolved early to a
>> +  branch name (referred to as $HEAD).  In other words, push the
>> +  current branch to update a branch with the same name on the pushing
>> +  side.
>
> As already pointed out, dropping everything before and including "In
> other words, " would be better.

I'd like to put it in the end, like Matthieu suggested.

> Also "push the current branch" is
> talking about the branch on the pushing side (you, the one who is
> running "git push"), and the side that is getting updated is at the
> receiving end, not "pushing side".

"receiving end" is better, yes.

> I think listing 'simple' at the end would be easier to read.  The
> above already swaps the order to make sure current and upstream are
> described before it, which is good.

As I pointed out, `simple` is just a composition of `current` and
`upstream`.  That's why it comes after them.  Also, I thought
`nothing`, `current`, `upstream` were fundamental: which is why they
come first.

> But I do not see a reason to move 'matching' which was next to
> 'nothing' here.

It's probably a personal bias, but I don't like matching at all :P

>> +* `matching` - push the refspec ":".  In other words, push all
>> +  branches having the same name in both ends, even if it means
>> +  non-fast-forward updates.  This is for those who prepare all the
>> +  branches into a publishable shape and then push them out with a
>> +  single command.  Dangerous, and inappropriate unless you are the
>> +  only person updating your push destination.
>
> It was already pointed out that unnecessary negativity needs to be
> fixed, but more importantly the above "Dangerous" is not even
> correct.

Okay, I'll explain my bias:

I have _never_ lost data with git: the reflog is my close companion.
There is one recent instance where I *lost* data, and I was very
unhappy about it.  I work on multiple machines, and not all local
branches are always synced with the upstream branches: when I switch
to a local branch to work on the topic, I notice my prompt and get it
to '=' before starting work.  In this one instance, I was developing
@{push} and toying around with various push.default settings.  I'd
forgotten that I'd set push.default to matching and issued my usual
push to update my branch.  This resulted in a huge number of my
branches getting rewound, and I could do nothing about it!

We've mentioned that pull --rebase is _dangerous_ (with the underline
for effect) in our documentation, when it's trivial to recover from
it: git reset --hard @{1}.  Yet, when I mention matching being
dangerous, I'm accused of being unnecessarily negative :\

In my opinion, `matching` is a bad and unpredictable setting, and
deserves last place on the list.  `nothing` deserves first place,
because it is the most predictable and least surprising.  Do not
mistake my stance for "matching is not useful"; ofcourse it is useful,
and I sometimes use it myself using ":" explicitly.

>> -+
>> -The `simple`, `current` and `upstream` modes are for those who want to
>> -push out a single branch after finishing work, even when the other
>> -branches are not yet ready to be pushed out. If you are working with
>> -other people to push into the same shared repository, you would want
>> -to use one of these.
>
> And I do not see a reason to drop this part, especially the first
> sentence.

I thought we should make it clear in the `matching` section; after
all, it's just `matching` versus everything else.