Re: [PATCH v4 2/8] SubmittingPatches: clarify 'git-contacts' location

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2024, 17:09 UTC
Message-ID: <xmqqsezqmrtd.fsf@gitster.g>
In-Reply-To: <c43de19d867cb5e63fe6689b2b7d645dc4741950.1712878339.git.gitgitgadget@gmail.com>

"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Linus Arver <linusa@google.com>
>
> Use a dash ("git-contacts", not "git contacts") because the script
> is not a core builtin command that is compiled into the `git` binary.

Pedantic, but "git mergetool" is how it is spelled even though it is not a core builtin command and is not compiled into the binary. The reason why "git-contacts" is better is because we do not install it to be usable by user's "git".

    ... because the script is not installed as part of "git"
    toolset.

An obvious alternative of course is to promote "contacts" out of "contrib/" and install it as part of the standard toolset. I gave a brief scan of the script and did not find anything (other than "only the recent 5 years worth of history matters") that is too specific to our project and I suspect it should do a reasonable job when run in any repository/working tree of a git-managed project.

But it is outside the scope of this series. I'd still welcome the thought to do that after the dust settles, though.

Show 6 quoted lines

> This also puts the script on one line, which should make it easier to
> grep for with a loose search query, such as
>
>     $ git grep git.contacts Documentation
>
> . Also add a footnote to describe where the script could actually be

Let's drop ". "; it may leave the previous sentence appear hanging unterminated, but the capital A that begins a new sentence is a good enough sign that we finished the previous sentence, isn't it?

Show 14 quoted lines

> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index e734a3f0f17..8b6e4bf0300 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -493,9 +493,16 @@ security relevant should not be submitted to the public mailing list
>  mentioned below, but should instead be sent privately to the Git
>  Security mailing list{security-ml-ref}.
>  
> +:contrib-scripts: footnoteref:[contrib-scripts,Scripts under `contrib/` are not +
> +part of the core `git` binary and must be called separately. Consult your +
> +package manager to determine where it is located. For example&#44; on Ubuntu-based +
> +systems it could be installed under +
> +`/usr/share/doc/git/contrib/contacts/git-contacts` and may need to be called +
> +with `perl ...` if it does not have the executable bit set.]

I wouldn't call anything in /usr/share/doc/ "installed", though.

In the context of _this_ document where the user is working on _git_ project towards submitting patches to _us_, it is far simpler to drop the above paragraph and tell them how to run the script in contrib/, e.g.

    $ perl contrib/contacts/git-contacts <args>...

without hinting there is anything platform/distro specific, and instead to have them all work from our sources.

I am assuming that any user who are reading this part of the document would have a reasonably recent version of our sources checked out (after all, they already have a patch or two to send but they are learning the way to find whom to send them to).