Re: [PATCH] doc: use 'ref' instead of 'commit' for merge-base arguments

From: Bryan Turner <bturner@atlassian.com>
To: Takuya N via GitGitGadget <gitgitgadget@gmail.com>
Cc: Git Users <git@vger.kernel.org>,
	Takuya Noguchi <takninnovationresearch@gmail.com>
Subject: Re: [PATCH] doc: use 'ref' instead of 'commit' for merge-base arguments
Date: Tue, 3 Mar 2020 00:32:02 -0700	[thread overview]
Message-ID: <CAGyf7-Ez1Fx-VUVmDWxRxLaZcU+Tu4kZ+F2+0uNKkx2oftjEJQ@mail.gmail.com> (raw)
In-Reply-To: <pull.719.git.git.1583220197856.gitgitgadget@gmail.com>

On Tue, Mar 3, 2020 at 12:23 AM Takuya N via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Takuya Noguchi <takninnovationresearch@gmail.com>
>
> The notation <commit> can be misunderstandable only for commit SHA1,
> but merge-base accepts any commit references. Like reflog, the name of
> arguments should be <ref> instead of <commit>.

To me, this change goes too far in the opposite direction: Now it
sounds like the command only accepts refs, when it actually accepts
any "commit-ish"--i.e., anything that can be coerced to a commit.
("git worktree" uses this term in its usage for "add", for example.)

At the same time, it doesn't seem like this change goes far enough.
"git merge"'s documentation, for example, is still using "<commit>".
Why is it important that "git merge-base" mention refs, but not that
"git merge" do so?

(Pardon the outburst from the peanut gallery)

>
> Signed-off-by: Takuya Noguchi <takninnovationresearch@gmail.com>
> ---
>     doc: use 'ref' instead of 'commit' for merge-base arguments
>
>     The notation <commit> can be misunderstandable only for commit SHA1, but
>     merge-base accepts any commit references. Like reflog, the name of
>     arguments should be <ref> rather than <commit>.
>
>     Signed-off-by: Takuya Noguchi takninnovationresearch@gmail.com
>     [takninnovationresearch@gmail.com]
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-719%2Ftnir%2Fmerge-base-supporting-refs-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-719/tnir/merge-base-supporting-refs-v1
> Pull-Request: https://github.com/git/git/pull/719
>
>  Documentation/git-merge-base.txt | 10 +++++-----
>  builtin/merge-base.c             | 12 ++++++------
>  2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt
> index 2d944e0851f..b87528ef269 100644
> --- a/Documentation/git-merge-base.txt
> +++ b/Documentation/git-merge-base.txt
> @@ -9,11 +9,11 @@ git-merge-base - Find as good common ancestors as possible for a merge
>  SYNOPSIS
>  --------
>  [verse]
> -'git merge-base' [-a|--all] <commit> <commit>...
> -'git merge-base' [-a|--all] --octopus <commit>...
> -'git merge-base' --is-ancestor <commit> <commit>
> -'git merge-base' --independent <commit>...
> -'git merge-base' --fork-point <ref> [<commit>]
> +'git merge-base' [-a|--all] <ref> <ref>...
> +'git merge-base' [-a|--all] --octopus <ref>...
> +'git merge-base' --is-ancestor <ref> <ref>
> +'git merge-base' --independent <ref>...
> +'git merge-base' --fork-point <ref> [<ref>]
>
>  DESCRIPTION
>  -----------
> diff --git a/builtin/merge-base.c b/builtin/merge-base.c
> index e3f8da13b69..910916ae0ec 100644
> --- a/builtin/merge-base.c
> +++ b/builtin/merge-base.c
> @@ -29,11 +29,11 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
>  }
>
>  static const char * const merge_base_usage[] = {
> -       N_("git merge-base [-a | --all] <commit> <commit>..."),
> -       N_("git merge-base [-a | --all] --octopus <commit>..."),
> -       N_("git merge-base --independent <commit>..."),
> -       N_("git merge-base --is-ancestor <commit> <commit>"),
> -       N_("git merge-base --fork-point <ref> [<commit>]"),
> +       N_("git merge-base [-a | --all] <ref> <ref>..."),
> +       N_("git merge-base [-a | --all] --octopus <ref>..."),
> +       N_("git merge-base --independent <ref>..."),
> +       N_("git merge-base --is-ancestor <ref> <ref>"),
> +       N_("git merge-base --fork-point <ref1> [<ref2>]"),
>         NULL
>  };
>
> @@ -158,7 +158,7 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
>                 OPT_CMDMODE(0, "is-ancestor", &cmdmode,
>                             N_("is the first one ancestor of the other?"), 'a'),
>                 OPT_CMDMODE(0, "fork-point", &cmdmode,
> -                           N_("find where <commit> forked from reflog of <ref>"), 'f'),
> +                           N_("find where <ref2> forked from reflog of <ref1>"), 'f'),
>                 OPT_END()
>         };
>
>
> base-commit: 2d2118b814c11f509e1aa76cb07110f7231668dc
> --
> gitgitgadget