Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 5, 2016, 18:07 UTC
Message-ID: <xmqq60p6pt4k.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <e74ae8afc1bfc4cd9161ccaa56d926a89439551e.1475678515.git.sorganov@gmail.com>

sorganov@gmail.com writes:

Show 41 quoted lines

> 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 ;-)

Show 9 quoted lines

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

Show 5 quoted lines

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

Show 5 quoted lines

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