All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Sebastian Schuberth <sschuberth@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Johannes Sixt <j6t@kdbg.org>,
	sorganov@gmail.com
Subject: Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does
Date: Mon, 30 Mar 2015 13:23:49 -0700	[thread overview]
Message-ID: <xmqqoanaw7xm.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqtwx2w937.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Mon, 30 Mar 2015 12:58:52 -0700")

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

>> Ignoring a merge sounds like dropping the merge commit and all side
>> branches it merges from history.
>
> ... Yes to _some_ people (including you, but not me).  And that is
> why we are trying to improve the text in the documentation, no?

Let's try this again as I do not think there is no need to argue in
the end.

We agree that this is a good patch:

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index d728030..47984e8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -362,7 +362,9 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`.
 
 -p::
 --preserve-merges::
-	Instead of ignoring merges, try to recreate them.
+	Recreate merge commits instead of flattening the history by replaying
+	commits a merge commit introduces. Merge conflict resolutions or manual
+	amendments to merge commits are not preserved.
 +
 This uses the `--interactive` machinery internally, but combining it
 with the `--interactive` option explicitly is generally not a good

The primary reason is "ignoring merges" can be read in two ways.

 - Some people, including the ones who wrote the original text (and
   also those who haven't complained to the original text since 2008
   because their reading of the phrase was the same as those who
   wrote it), read it as "We will normally replay what appears in
   the output from 'git log --no-merges', losing what was merged
   where into what.  This option instead keeps the shape of the
   history by considering where the merge commits sit in the
   original history".

 - It can however be read in a different way, namely, "We will
   replay what appears in 'git log --first-parent --no-merges'
   output, ignoring all the side branches".  Some other people,
   including you, find that inconsistent with what is actually
   implemented as a normal rebase.

And we agree the updated text is a good change, because it does not
say "ignore merge" anywhere to invite such a confusion.

I am only reacting to your description to justify that change, which
was:

> Ignoring a merge sounds like ignoring the changes a merge commit
> introduces altogether, as if the merge commit was skipped or dropped from
> history. But that is not what happens if "-p" is not specified.

And we agree that the first part of the sentence, up to
"... altogether", is good.

I am saying "as if" part is reintroducing the confusion.

    as if the merge commit was skipped or dropped from history.

To those who did not interpret the existing "ignoring merges", which
meant to be read as "replay the output of 'log --no-merges'
sequencially", the above exactly says "replay the output of 'log
--no-merges'".  That is the reason why they were happy with the
current text, without seeing how it can be read differently.

To them (including me), the justfication is saying "... sounds like
X, as if Y, but it is wrong" using Y that is completely opposite of
X, even they would agree that X is a wrong interpretation caused by
a non-optimal phrasing.

That is why I suggested to avoid "merge commit was skipped or
dropped" in the description and instead say "as if the side branch
that was merged was skipped or dropped".

The following may be agreeable to both camps of people, those who
read "ignoring merge" to mean "log --no-merges" and those who read
it to mean "log --first-parent --no-merges", I would think.

-- >8 --
From: Sebastian Schuberth <sschuberth@gmail.com>
Date: Mon, 30 Mar 2015 11:29:46 +0200
Subject docs: clarify what git-rebase's "-p" / "--preserve-merges" does

Ignoring a merge can be read as ignoring the changes a merge commit
introduces altogether, as if the entire side branch the merge commit
merged was removed from the history.  But that is not what happens
if "-p" is not specified.  What happens is that the individual
commits a merge commit introduces are replayed in order, and only
any possible merge conflict resolutions or manual amendments to the
merge commit are ignored.

Get this straight in the docs.

Also, do not say that merge commits are *tried* to be recreated. As that is
true almost everywhere it is better left unsaid.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 Documentation/git-rebase.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index d728030..47984e8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -362,7 +362,9 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`.
 
 -p::
 --preserve-merges::
-	Instead of ignoring merges, try to recreate them.
+	Recreate merge commits instead of flattening the history by replaying
+	commits a merge commit introduces. Merge conflict resolutions or manual
+	amendments to merge commits are not preserved.
 +
 This uses the `--interactive` machinery internally, but combining it
 with the `--interactive` option explicitly is generally not a good

  reply	other threads:[~2015-03-30 20:24 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-26 13:04 [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does Sebastian Schuberth
2015-03-26 18:18 ` Junio C Hamano
2015-03-26 20:28   ` Sebastian Schuberth
2015-03-26 20:55     ` Junio C Hamano
2015-03-26 21:17   ` Sergey Organov
2015-03-26 21:41     ` Johannes Sixt
2015-03-31  9:13       ` Sergey Organov
2015-03-31 16:28         ` Junio C Hamano
2015-03-31 17:03           ` Sergey Organov
2015-03-31 17:04           ` Junio C Hamano
2015-04-01 11:27             ` Sergey Organov
2015-04-01 17:03               ` Junio C Hamano
2015-04-02  9:53                 ` Sergey Organov
2015-03-30  9:29   ` Sebastian Schuberth
2015-03-30 17:23     ` Junio C Hamano
2015-03-30 19:42       ` Sebastian Schuberth
2015-03-30 19:58         ` Junio C Hamano
2015-03-30 20:23           ` Junio C Hamano [this message]
2015-03-30 21:09             ` Sebastian Schuberth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqoanaw7xm.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=sorganov@gmail.com \
    --cc=sschuberth@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.