git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: sorganov@gmail.com
Cc: git@vger.kernel.org
Subject: Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION
Date: Wed, 05 Oct 2016 11:07:07 -0700	[thread overview]
Message-ID: <xmqq60p6pt4k.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <e74ae8afc1bfc4cd9161ccaa56d926a89439551e.1475678515.git.sorganov@gmail.com> (sorganov@gmail.com's message of "Wed, 5 Oct 2016 17:46:23 +0300")

sorganov@gmail.com writes:

> From: Sergey Organov <sorganov@gmail.com>
>
> Old description had a few problems:
>
> - sounded as if commits have changes
>
> - stated that changes are taken since some "divergence point"
>   that was not defined.
>
> New description rather uses "common ancestor" and "merge base",
> definitions of which are easily discoverable in the rest of GIT
> documentation.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/git-merge.txt | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index cc0329d..351b8fc 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -16,11 +16,16 @@ SYNOPSIS
>  
>  DESCRIPTION
>  -----------
> -Incorporates changes from the named commits (since the time their
> -histories diverged from the current branch) into the current
> -branch.  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.
> +
> +Incorporates changes that lead to the named commits into the current
> +branch, and joins corresponding histories. The best common ancestor of
> +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.

Content change together with re-flowing the text makes it more
costly than necessary to review a change like this.  Please avoid
doing so in your future patches.

I like what the updated description says very much.  I however
wonder if "and can be used by hand..." is still appropriate, or
needs a bit of modernizing.  It feels a bit awkward by making it
sound as if 'git merge' is primarily an implementation detail of
'git pull' but it can also be used as the first-class command, which
used to be the case in the old days back when "git pull . other" was
also perfectly good way to merge the 'other' branch from your own
repository, but I think your update is meant to clarify that we no
longer live in that old world ;-)

> @@ -31,11 +36,11 @@ Assume the following history exists and the current branch is
>      D---E---F---G master
>  ------------
>  
> -Then "`git merge topic`" will replay the changes made on the
> -`topic` branch since it diverged from `master` (i.e., `E`) until
> -its current commit (`C`) on top of `master`, and record the result
> -in a new commit along with the names of the two parent commits and
> -a log message from the user describing the changes.

> -Then "`git merge topic`" will replay the changes made on the `topic`
> -branch since it diverged from `master` (i.e., `E`) until its current
> -commit (`C`) on top of `master`, and record the result in a new commit
> -along with the names of the two parent commits and a log message from
> -the user describing the changes.

> +Then "`git merge topic`" will replay the changes made on the `topic`
> +branch since it diverged from `master` (i.e., `E`) until its current
> +commit (`C`) on top of `master`, and record the result in a new commit
> +along with references to the two parent commits and a log message from
> +the user describing the changes.

Content change together with re-flowing the text makes it more
costly than necessary to review a change like this.  Please avoid
doing so in your future patches.

I had to re-flow the original you removed to match how you flowed in
the updated one and stare at it for a while to spot that the only
change was to rephrase "the names of the parents" to "references to
the parents".  I do not know if the updated phrasing is better.  The
"name" in the original was meant to be a short-hand for "object name",
and I would support a change to spell it out to clarify; "reference"
can be a vague word that can mean different things in Git, and when
the word is given without context, most Git people would think that
the word refers to "refs", but that is definitely not what the new
commit records, so...

  parent reply	other threads:[~2016-10-05 18:07 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-05 14:46 [PATCH 0/6] git-merge: a few documentation improvements sorganov
2016-10-05 14:46 ` [PATCH 1/6] git-merge: clarify "usage" by adding "-m <msg>" sorganov
2016-10-05 17:46   ` Junio C Hamano
2016-10-05 20:41     ` Sergey Organov
2016-10-05 14:46 ` [PATCH 2/6] Documentation/git-merge.txt: remove list of options from SYNOPSIS sorganov
2016-10-05 17:47   ` Junio C Hamano
2016-10-05 21:03     ` Sergey Organov
2016-10-05 14:46 ` [PATCH 3/6] Documentation/git-merge.txt: fix SYNOPSIS of obsolete form to include options sorganov
2016-10-05 14:46 ` [PATCH 4/6] Documentation/git-merge.txt: improve short description in NAME sorganov
2016-10-05 17:52   ` Junio C Hamano
2016-10-05 21:01     ` Sergey Organov
2016-10-05 17:55   ` Jeff King
2016-10-05 20:44     ` Sergey Organov
2016-10-05 14:46 ` [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION sorganov
2016-10-05 16:58   ` Jakub Narębski
2016-10-05 20:01     ` Junio C Hamano
2016-10-05 21:27     ` Sergey Organov
2016-10-06 13:21     ` Sergey Organov
2016-10-05 18:07   ` Junio C Hamano [this message]
2016-10-05 21:24     ` Sergey Organov
2016-10-05 21:41       ` Junio C Hamano
2016-10-06 12:30         ` Sergey Organov
2016-10-06 17:46           ` Junio C Hamano
2016-10-07 13:13             ` Sergey Organov
2016-10-05 14:46 ` [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull sorganov
2016-10-05 18:57   ` Junio C Hamano
2016-10-05 21:34     ` Sergey Organov
2016-10-05 21:43       ` Junio C Hamano
2016-10-06 12:39         ` Sergey Organov
2016-10-06 18:06           ` Junio C Hamano
2016-10-07 11:45             ` Sergey Organov

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq60p6pt4k.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=sorganov@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).