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

[Linus Arver <linusa@google.com>]

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

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

Noted; I will use this wording.

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

Ack. Ideally we would translate it to C (or, dare I say, into a C lib +
some other higher level language, Perl or otherwise), but I agree that's
for another series.

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

Will do.

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

In hindsight I agree. From a quick Google search [1] it looks like your
style is what's preferred in academia also.

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

Indeed. One small change is that the script already has the execute bit
set so I can drop `perl` as $0 (the execute bit is removed when it is
copied into /usr/share/... on my system).

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

Agreed.

[1] https://camosun.libguides.com/Chicago-17thEd/quotations