Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Sergey Organov]

Junio C Hamano  writes:

> Sergey Organov  writes:
>
>> Ah, I now see. I tried to keep the text intact as much as possible, and
>> only split it into description and a note. Well, how about this then:
>
> Much better than your earlier patch, but I am not sure if the
> updated one is that much better compared to the original.

It's not intended to be much better. It is aimed at single simple
target: get rid of git-pull from descriptions of operations of
git-merge.

I'd just remove those git-pull reference, the only one that is left
after the patch, but it looks like git-merge needs an excuse to have
fast-forward on by default, and that excuse is the common git-pull case.

[I'd prefer 'git-merge --ff' were called from 'git-pull' and --no-ff be
the default for git-merge, but that's not the case, so I left the
reference to git-pull intact.]

>
> The pre- and post- state of this "how about this" patch essentially
> say the same thing, and I suspect that the primary reason why you
> think the post- state is easier to read is because you wrote it,
> while the reason why I do not see much difference is because I
> didn't write the updated one ;-).
>
> I do find "In this case, ... store the combined history" in the
> original a bit awkward to read, but most of that awkardness is
> inherited by the updated text.  It may benefit from hinting why a
> new commit is not needed a bit stronger.  Here is my attempt:
>
> When the commit we are merging is a descendant of the current
> HEAD, the history leading to the named commit can be, and by
> default is, taken as the combined history of the two.  Our
> history is "fast forwarded" to their history by updating `HEAD`
> along with the index to point at the named commit.
>
> This often happens when you are following along somebody else's
> work via "git pull" without doing your own development.
>
> I think the awkwardness I felt in the original and your version is
> gone from the above attempt, but I doubt that it is better over
> either of them in any other way.

This is entirely different matter, and should be a subject of another
patch, if any. My patch meant to only address git-pull references, with
as few changes as possible.

-- Sergey

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Junio C Hamano]

Sergey Organov  writes:

> Ah, I now see. I tried to keep the text intact as much as possible, and
> only split it into description and a note. Well, how about this then:

Much better than your earlier patch, but I am not sure if the
updated one is that much better compared to the original.

The pre- and post- state of this "how about this" patch essentially
say the same thing, and I suspect that the primary reason why you
think the post- state is easier to read is because you wrote it,
while the reason why I do not see much difference is because I
didn't write the updated one ;-).

I do find "In this case, ... store the combined history" in the
original a bit awkward to read, but most of that awkardness is
inherited by the updated text.  It may benefit from hinting why a
new commit is not needed a bit stronger.  Here is my attempt:

When the commit we are merging is a descendant of the current
HEAD, the history leading to the named commit can be, and by
default is, taken as the combined history of the two.  Our
history is "fast forwarded" to their history by updating `HEAD`
along with the index to point at the named commit.

This often happens when you are following along somebody else's
work via "git pull" without doing your own development.

I think the awkwardness I felt in the original and your version is
gone from the above attempt, but I doubt that it is better over
either of them in any other way.

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index b758d55..479400f 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -135,15 +135,17 @@ will exit early with the message "Already up-to-date."
>  FAST-FORWARD MERGE
>  --
>  
> -Often the current branch head is an ancestor of the named commit.
> -This is the most common case especially when invoked from 'git
> -pull': you are tracking an upstream repository, you have committed
> -no local changes, and now you want to update to a newer upstream
> -revision.  In this case, a new commit is not needed to store the
> +When current branch head is an ancestor of the named commit,
> +a new commit is not needed to store the
>  combined history; instead, the `HEAD` (along with the index) is
>  updated to point at the named commit, without creating an extra
>  merge commit.
>  
> +This is very common case when 'git merge' is invoked from 'git
> +pull': you are tracking an upstream repository, you have committed
> +no local changes, and now you want to update to a newer upstream
> +revision.  
> +
>  This behavior can be suppressed with the `--no-ff` option.
>  
>  TRUE MERGE
>
> -- Sergey

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Sergey Organov]

Junio C Hamano  writes:

> Sergey Organov  writes:
>
>> So I believe this change is inline with the rest of the patch. The
>> reference to git-pull (if it remains) should be a side-note, not part of
>> explanation of operation.
>
> Not really.  The thing is, "This is the most common" needs to be
> close to "Often...".  "git merge" directly invoked by the end user
> is much less likely to encounter a fast forward situation; getting
> invoked indirectly by "git pull" makes it common.

Ah, I now see. I tried to keep the text intact as much as possible, and
only split it into description and a note. Well, how about this then:

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d55..479400f 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -135,15 +135,17 @@ will exit early with the message "Already up-to-date."
 FAST-FORWARD MERGE
 --
 
-Often the current branch head is an ancestor of the named commit.
-This is the most common case especially when invoked from 'git
-pull': you are tracking an upstream repository, you have committed
-no local changes, and now you want to update to a newer upstream
-revision.  In this case, a new commit is not needed to store the
+When current branch head is an ancestor of the named commit,
+a new commit is not needed to store the
 combined history; instead, the `HEAD` (along with the index) is
 updated to point at the named commit, without creating an extra
 merge commit.
 
+This is very common case when 'git merge' is invoked from 'git
+pull': you are tracking an upstream repository, you have committed
+no local changes, and now you want to update to a newer upstream
+revision.  
+
 This behavior can be suppressed with the `--no-ff` option.
 
 TRUE MERGE

-- Sergey

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Junio C Hamano]

Sergey Organov  writes:

> So I believe this change is inline with the rest of the patch. The
> reference to git-pull (if it remains) should be a side-note, not part of
> explanation of operation.

Not really.  The thing is, "This is the most common" needs to be
close to "Often...".  "git merge" directly invoked by the end user
is much less likely to encounter a fast forward situation; getting
invoked indirectly by "git pull" makes it common.

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Sergey Organov]

Junio C Hamano  writes:

> sorga...@gmail.com writes:

[...]

>> @@ -138,14 +133,15 @@ will exit early with the message "Already up-to-date."
>>  FAST-FORWARD MERGE
>>  --
>>  
>> -Often the current branch head is an ancestor of the named commit.
>> +Often the current branch head is an ancestor of the named commit.  In
>> +this case, a new commit is not needed to store the combined history;
>> +instead, the `HEAD` (along with the index) is updated to point at the
>> +named commit, without creating an extra merge commit.
>> +
>>  This is the most common case especially when invoked from 'git
>>  pull': you are tracking an upstream repository, you have committed
>>  no local changes, and now you want to update to a newer upstream
>> -revision.  In this case, a new commit is not needed to store the
>> -combined history; instead, the `HEAD` (along with the index) is
>> -updated to point at the named commit, without creating an extra
>> -merge commit.
>> +revision.
>
> I am not sure if the post-image of this hunk is better than the
> original.

That's what I've tried to explain in the description of the patch:

"No awareness of git-pull is required to understand git-merge operation,
so leave reference to git-pull only where it actually makes sense, in
the description of fast-forward merges, and only as clarification of
when this merging behaviour is mostly useful."

So I believe this change is inline with the rest of the patch. The
reference to git-pull (if it remains) should be a side-note, not part of
explanation of operation.

-- Sergey

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Junio C Hamano]

sorga...@gmail.com writes:

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 351b8fc..ba5fb0a 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -23,10 +23,6 @@ named commits and the current branch, called "merge base", 
> is
>  calculated, and then net changes taken from the merge base to
>  the named commits are applied.
>  
> -This command is used by 'git pull' to incorporate changes from another
> -repository, and can be used by hand to merge changes from one branch
> -into another.
> -

Good.

> @@ -119,18 +115,17 @@ of `git fetch` for merging are merged to the current 
> branch.
>  PRE-MERGE CHECKS
>  
>  
> -Before applying outside changes, you should get your own work in
> -good shape and committed locally, so it will not be clobbered if
> -there are conflicts.  See also linkgit:git-stash[1].
> -'git pull' and 'git merge' will stop without doing anything when
> -local uncommitted changes overlap with files that 'git pull'/'git
> -merge' may need to update.
> +Before applying outside changes, you should get your own work in good
> +shape and committed locally, so it will not be clobbered if there are
> +conflicts. See also linkgit:git-stash[1]. 'git merge' will stop
> +without doing anything when local uncommitted changes overlap with
> +files that 'git merge' may need to update.
>  
> -To avoid recording unrelated changes in the merge commit,
> -'git pull' and 'git merge' will also abort if there are any changes
> -registered in the index relative to the `HEAD` commit.  (One
> -exception is when the changed index entries are in the state that
> -would result from the merge already.)
> +To avoid recording unrelated changes in the merge commit, 'git merge'
> +will also abort if there are any changes registered in the index
> +relative to the `HEAD` commit. (One exception is when the changed
> +index entries are in the state that would result from the merge
> +already.)

OK, so "git pull and git merge" have been updated to say "git
merge" and there is no other change.  Looks good.

Please do not re-flow and change in the same commit, by the way.

> @@ -138,14 +133,15 @@ will exit early with the message "Already up-to-date."
>  FAST-FORWARD MERGE
>  --
>  
> -Often the current branch head is an ancestor of the named commit.
> +Often the current branch head is an ancestor of the named commit.  In
> +this case, a new commit is not needed to store the combined history;
> +instead, the `HEAD` (along with the index) is updated to point at the
> +named commit, without creating an extra merge commit.
> +
>  This is the most common case especially when invoked from 'git
>  pull': you are tracking an upstream repository, you have committed
>  no local changes, and now you want to update to a newer upstream
> -revision.  In this case, a new commit is not needed to store the
> -combined history; instead, the `HEAD` (along with the index) is
> -updated to point at the named commit, without creating an extra
> -merge commit.
> +revision.

I am not sure if the post-image of this hunk is better than the
original.

[PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[sorganov]

From: Sergey Organov 

No awareness of git-pull is required to understand git-merge operation,
so leave reference to git-pull only where it actually makes sense, in
the description of fast-forward merges, and only as clarification of
when this merging behaviour is mostly useful.

Other references to git-pull are likely just a historical leftover
that are now neither required nor clarify anything. Besides, git-pull
may use rebase rather than merge, so it's also technically wrong to
say, unconditionally, that git-pull uses git-merge.

Overall, let git-pull description refer to git-merge where
appropriate, and not vice versa.

Signed-off-by: Sergey Organov 
---
 Documentation/git-merge.txt | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 351b8fc..ba5fb0a 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -23,10 +23,6 @@ named commits and the current branch, called "merge base", is
 calculated, and then net changes taken from the merge base to
 the named commits are applied.
 
-This command is used by 'git pull' to incorporate changes from another
-repository, and can be used by hand to merge changes from one branch
-into another.
-
 Assume the following history exists and the current branch is
 "`master`":
 
@@ -119,18 +115,17 @@ of `git fetch` for merging are merged to the current 
branch.
 PRE-MERGE CHECKS
 
 
-Before applying outside changes, you should get your own work in
-good shape and committed locally, so it will not be clobbered if
-there are conflicts.  See also linkgit:git-stash[1].
-'git pull' and 'git merge' will stop without doing anything when
-local uncommitted changes overlap with files that 'git pull'/'git
-merge' may need to update.
+Before applying outside changes, you should get your own work in good
+shape and committed locally, so it will not be clobbered if there are
+conflicts. See also linkgit:git-stash[1]. 'git merge' will stop
+without doing anything when local uncommitted changes overlap with
+files that 'git merge' may need to update.
 
-To avoid recording unrelated changes in the merge commit,
-'git pull' and 'git merge' will also abort if there are any changes
-registered in the index relative to the `HEAD` commit.  (One
-exception is when the changed index entries are in the state that
-would result from the merge already.)
+To avoid recording unrelated changes in the merge commit, 'git merge'
+will also abort if there are any changes registered in the index
+relative to the `HEAD` commit. (One exception is when the changed
+index entries are in the state that would result from the merge
+already.)
 
 If all named commits are already ancestors of `HEAD`, 'git merge'
 will exit early with the message "Already up-to-date."
@@ -138,14 +133,15 @@ will exit early with the message "Already up-to-date."
 FAST-FORWARD MERGE
 --
 
-Often the current branch head is an ancestor of the named commit.
+Often the current branch head is an ancestor of the named commit.  In
+this case, a new commit is not needed to store the combined history;
+instead, the `HEAD` (along with the index) is updated to point at the
+named commit, without creating an extra merge commit.
+
 This is the most common case especially when invoked from 'git
 pull': you are tracking an upstream repository, you have committed
 no local changes, and now you want to update to a newer upstream
-revision.  In this case, a new commit is not needed to store the
-combined history; instead, the `HEAD` (along with the index) is
-updated to point at the named commit, without creating an extra
-merge commit.
+revision.
 
 This behavior can be suppressed with the `--no-ff` option.
 
-- 
2.10.0.1.g57b01a3