diff mbox series

docs: recommend using contrib/contacts/git-contacts

Message ID pull.1704.git.1712017205754.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series docs: recommend using contrib/contacts/git-contacts | expand

Commit Message

Linus Arver April 2, 2024, 12:20 a.m. UTC
From: Linus Arver <linusa@google.com>

Although we've had this script since 4d06402b1b (contrib: add
git-contacts helper, 2013-07-21), we don't mention it in our
introductory docs. Do so now.

Signed-off-by: Linus Arver <linusa@google.com>
---
    docs: recommend using contrib/contacts/git-contacts

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1704%2Flistx%2Freviewers-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1704/listx/reviewers-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1704

 Documentation/MyFirstContribution.txt | 3 +++
 Documentation/SubmittingPatches       | 4 ++++
 2 files changed, 7 insertions(+)


base-commit: c2cbfbd2e28cbe27c194d62183b42f27a6a5bb87

Comments

Patrick Steinhardt April 2, 2024, 6:28 a.m. UTC | #1
On Tue, Apr 02, 2024 at 12:20:05AM +0000, Linus Arver via GitGitGadget wrote:
> From: Linus Arver <linusa@google.com>
> 
> Although we've had this script since 4d06402b1b (contrib: add
> git-contacts helper, 2013-07-21), we don't mention it in our
> introductory docs. Do so now.
> 
> Signed-off-by: Linus Arver <linusa@google.com>
> ---
>     docs: recommend using contrib/contacts/git-contacts
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1704%2Flistx%2Freviewers-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1704/listx/reviewers-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1704
> 
>  Documentation/MyFirstContribution.txt | 3 +++
>  Documentation/SubmittingPatches       | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> index f06563e9817..eb1e27a82df 100644
> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -1116,6 +1116,9 @@ $ git send-email --to=target@example.com psuh/*.patch
>  NOTE: Check `git help send-email` for some other options which you may find
>  valuable, such as changing the Reply-to address or adding more CC and BCC lines.
>  
> +NOTE: Use `contrib/contacts/git-contacts` to get a list of reviewers you should
> +include in the CC list.
> +

Should we mention that the script can be passed to git-send-email(1) via
`--cc-cmd=`?

Thanks!

Patrick

>  NOTE: When you are sending a real patch, it will go to git@vger.kernel.org - but
>  please don't send your patchset from the tutorial to the real mailing list! For
>  now, you can send it to yourself, to make sure you understand how it will look.
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index e734a3f0f17..52d11ff510b 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -459,6 +459,10 @@ an explanation of changes between each iteration can be kept in
>  Git-notes and inserted automatically following the three-dash
>  line via `git format-patch --notes`.
>  
> +[[suggested-reviewers]]
> +Use `contrib/contacts/git-contacts` to get a list of reviewers you should
> +include in the CC list.
> +
>  [[attachment]]
>  Do not attach the patch as a MIME attachment, compressed or not.
>  Do not let your e-mail client send quoted-printable.  Do not let
> 
> base-commit: c2cbfbd2e28cbe27c194d62183b42f27a6a5bb87
> -- 
> gitgitgadget
>
Matthias Aßhauer April 3, 2024, 8:42 a.m. UTC | #2
On Tue, 2 Apr 2024, Linus Arver via GitGitGadget wrote:

> From: Linus Arver <linusa@google.com>
>
> Although we've had this script since 4d06402b1b (contrib: add
> git-contacts helper, 2013-07-21), we don't mention it in our
> introductory docs. Do so now.
>
> Signed-off-by: Linus Arver <linusa@google.com>
> ---
>    docs: recommend using contrib/contacts/git-contacts
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1704%2Flistx%2Freviewers-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1704/listx/reviewers-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1704
>
> Documentation/MyFirstContribution.txt | 3 +++
> Documentation/SubmittingPatches       | 4 ++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> index f06563e9817..eb1e27a82df 100644
> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -1116,6 +1116,9 @@ $ git send-email --to=target@example.com psuh/*.patch
> NOTE: Check `git help send-email` for some other options which you may find
> valuable, such as changing the Reply-to address or adding more CC and BCC lines.
>
> +NOTE: Use `contrib/contacts/git-contacts` to get a list of reviewers you should
> +include in the CC list.
> +
> NOTE: When you are sending a real patch, it will go to git@vger.kernel.org - but
> please don't send your patchset from the tutorial to the real mailing list! For
> now, you can send it to yourself, to make sure you understand how it will look.
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index e734a3f0f17..52d11ff510b 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -459,6 +459,10 @@ an explanation of changes between each iteration can be kept in
> Git-notes and inserted automatically following the three-dash
> line via `git format-patch --notes`.
>
> +[[suggested-reviewers]]
> +Use `contrib/contacts/git-contacts` to get a list of reviewers you should
> +include in the CC list.
> +

There is already a paragraph about this in Documentation/SubmittingPatches 
just a few paragraphs below.

> Send your patch with "To:" set to the mailing list, with "cc:" listing
> people who are involved in the area you are touching (the `git
> contacts` command in `contrib/contacts/` can help to
> identify them), to solicit comments and reviews.  Also, when you made
> trial merges of your topic to `next` and `seen`, you may have noticed
> work by others conflicting with your changes.  There is a good possibility
> that these people may know the area you are touching well.

Could we improve the existing paragraph instead of duplicating this 
information?

> [[attachment]]
> Do not attach the patch as a MIME attachment, compressed or not.
> Do not let your e-mail client send quoted-printable.  Do not let
>
> base-commit: c2cbfbd2e28cbe27c194d62183b42f27a6a5bb87
> -- 
> gitgitgadget
>
Matthias Aßhauer April 3, 2024, 10:11 a.m. UTC | #3
On Wed, 3 Apr 2024, Matthias Aßhauer wrote:

>

After sending my previous message I've noticed that all of the 
etu.univ-lyon1.fr recipients bounced with the 
message

> 550 5.5.0 Requested actions not taken as the mailbox is unavailable

After running https://etu.univ-lyon1.fr/ through a machine translation 
service it seems like that subdomain is used for mailboxes of current 
students, whereas staff like Matthieu get a mailbox on the main domain.
With Corentin, Nathan and Pablo presumably being former students, it's 
probably unlikely that these mailboxes will become active again.

Would it make sense to have a way to teach `git-contacts` to exclude a 
user defined list of known-bad recipient adresses? This could potentiallly 
be an extension of mailmap or a separate file.

>
> On Tue, 2 Apr 2024, Linus Arver via GitGitGadget wrote:
>
>> From: Linus Arver <linusa@google.com>
>> 
>> Although we've had this script since 4d06402b1b (contrib: add
>> git-contacts helper, 2013-07-21), we don't mention it in our
>> introductory docs. Do so now.
>> 
>> Signed-off-by: Linus Arver <linusa@google.com>
>> ---
>>    docs: recommend using contrib/contacts/git-contacts
>> 
>> Published-As: 
>> https://github.com/gitgitgadget/git/releases/tag/pr-1704%2Flistx%2Freviewers-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
>> pr-1704/listx/reviewers-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/1704
>> 
>> Documentation/MyFirstContribution.txt | 3 +++
>> Documentation/SubmittingPatches       | 4 ++++
>> 2 files changed, 7 insertions(+)
>> 
>> diff --git a/Documentation/MyFirstContribution.txt 
>> b/Documentation/MyFirstContribution.txt
>> index f06563e9817..eb1e27a82df 100644
>> --- a/Documentation/MyFirstContribution.txt
>> +++ b/Documentation/MyFirstContribution.txt
>> @@ -1116,6 +1116,9 @@ $ git send-email --to=target@example.com psuh/*.patch
>> NOTE: Check `git help send-email` for some other options which you may find
>> valuable, such as changing the Reply-to address or adding more CC and BCC 
>> lines.
>> 
>> +NOTE: Use `contrib/contacts/git-contacts` to get a list of reviewers you 
>> should
>> +include in the CC list.
>> +
>> NOTE: When you are sending a real patch, it will go to git@vger.kernel.org 
>> - but
>> please don't send your patchset from the tutorial to the real mailing list! 
>> For
>> now, you can send it to yourself, to make sure you understand how it will 
>> look.
>> diff --git a/Documentation/SubmittingPatches 
>> b/Documentation/SubmittingPatches
>> index e734a3f0f17..52d11ff510b 100644
>> --- a/Documentation/SubmittingPatches
>> +++ b/Documentation/SubmittingPatches
>> @@ -459,6 +459,10 @@ an explanation of changes between each iteration can 
>> be kept in
>> Git-notes and inserted automatically following the three-dash
>> line via `git format-patch --notes`.
>> 
>> +[[suggested-reviewers]]
>> +Use `contrib/contacts/git-contacts` to get a list of reviewers you should
>> +include in the CC list.
>> +
>
> There is already a paragraph about this in Documentation/SubmittingPatches 
> just a few paragraphs below.
>
>> Send your patch with "To:" set to the mailing list, with "cc:" listing
>> people who are involved in the area you are touching (the `git
>> contacts` command in `contrib/contacts/` can help to
>> identify them), to solicit comments and reviews.  Also, when you made
>> trial merges of your topic to `next` and `seen`, you may have noticed
>> work by others conflicting with your changes.  There is a good possibility
>> that these people may know the area you are touching well.
>
> Could we improve the existing paragraph instead of duplicating this 
> information?
>
>> [[attachment]]
>> Do not attach the patch as a MIME attachment, compressed or not.
>> Do not let your e-mail client send quoted-printable.  Do not let
>> 
>> base-commit: c2cbfbd2e28cbe27c194d62183b42f27a6a5bb87
>> -- 
>> gitgitgadget
>> 
>
Matthieu Moy April 3, 2024, 12:13 p.m. UTC | #4
On 4/3/24 12:11, Matthias Aßhauer wrote:
> 
> 
> On Wed, 3 Apr 2024, Matthias Aßhauer wrote:
> 
> After sending my previous message I've noticed that all of the etu.univ-lyon1.fr recipients bounced with the
> message
> 
>> 550 5.5.0 Requested actions not taken as the mailbox is unavailable
> 
> After running etu.univ-lyon1.fr&through a machine translation
> service it seems like that subdomain is used for mailboxes of current
> students,

Indeed. These are former students, who contributed to Git under my 
supervision (as teacher in the same university). I don't have their 
current email address.

> Would it make sense to have a way to teach `git-contacts` to exclude a
> user defined list of known-bad recipient adresses? This could potentiallly
> be an extension of mailmap or a separate file.

Sounds like a good idea, yes. At least, the current thread would be a 
good use-case for such feature.
Junio C Hamano April 3, 2024, 4:13 p.m. UTC | #5
Matthias Aßhauer <mha1993@live.de> writes:

> There is already a paragraph about this in
> Documentation/SubmittingPatches just a few paragraphs below.
>
>> Send your patch with "To:" set to the mailing list, with "cc:" listing
>> people who are involved in the area you are touching (the `git
>> contacts` command in `contrib/contacts/` can help to
>> identify them), to solicit comments and reviews.  Also, when you made
>> trial merges of your topic to `next` and `seen`, you may have noticed
>> work by others conflicting with your changes.  There is a good possibility
>> that these people may know the area you are touching well.
>
> Could we improve the existing paragraph instead of duplicating this
> information?

Absolutely.  I am glad that you were paying attention to the
question that triggered this patch ;-)

We may want to add to coding guidelines to try avoiding to word wrap
a line in the middle of a multi-word phrase.  If such a rule were
followed,

    $ git grep git.contacts Documentation

would easily had found the existing passage.

Thanks.
Junio C Hamano April 3, 2024, 4:39 p.m. UTC | #6
Matthias Aßhauer <mha1993@live.de> writes:

> Would it make sense to have a way to teach `git-contacts` to exclude a
> user defined list of known-bad recipient adresses? This could
> potentiallly be an extension of mailmap or a separate file.

The contacts script already uses "check-mailmap".  

Unfortunately it only uses the default mailmap, which may not be
suitable for our purpose here, but it shouldn't be too hard to run
"git -c mailmap.file=<custom> check-mailmap", ship a custom mailmap
file with the contacts script to map defunct addresses to something
that is clearly invalid, and then filter them out from the output.

We want to add a mechanism to allow "including" another mailmap, so
that "../../.mailmap" is included from contrib/contacts/mailmap or
something like that.

On the other hand, if we want to use our primary mailmap to also
mark the defunct addresses, then we do not have to do anything
special.  Mark these defunct addresses to the primary mailmap to
map them to "$HumanReadableName <$name@defunct.invalid>" and then
doing something like the attached.

diff --git i/contrib/contacts/git-contacts w/contrib/contacts/git-contacts
index 85ad732fc0..00e77c4125 100755
--- i/contrib/contacts/git-contacts
+++ w/contrib/contacts/git-contacts
@@ -197,6 +197,7 @@ $contacts = mailmap_contacts($contacts);
 
 my $ncommits = scalar(keys %commits);
 for my $contact (keys %$contacts) {
+	next if $contact =~ /\@defunct.invalid>$/;
 	my $percent = $contacts->{$contact} * 100 / $ncommits;
 	next if $percent < $min_percent;
 	print "$contact\n";
Linus Arver April 4, 2024, 8 p.m. UTC | #7
Patrick Steinhardt <ps@pks.im> writes:

> On Tue, Apr 02, 2024 at 12:20:05AM +0000, Linus Arver via GitGitGadget wrote:
>> From: Linus Arver <linusa@google.com>
>> 
>> Although we've had this script since 4d06402b1b (contrib: add
>> git-contacts helper, 2013-07-21), we don't mention it in our
>> introductory docs. Do so now.
>> 
>> Signed-off-by: Linus Arver <linusa@google.com>
>> ---
>>     docs: recommend using contrib/contacts/git-contacts
>> 
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1704%2Flistx%2Freviewers-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1704/listx/reviewers-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/1704
>> 
>>  Documentation/MyFirstContribution.txt | 3 +++
>>  Documentation/SubmittingPatches       | 4 ++++
>>  2 files changed, 7 insertions(+)
>> 
>> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
>> index f06563e9817..eb1e27a82df 100644
>> --- a/Documentation/MyFirstContribution.txt
>> +++ b/Documentation/MyFirstContribution.txt
>> @@ -1116,6 +1116,9 @@ $ git send-email --to=target@example.com psuh/*.patch
>>  NOTE: Check `git help send-email` for some other options which you may find
>>  valuable, such as changing the Reply-to address or adding more CC and BCC lines.
>>  
>> +NOTE: Use `contrib/contacts/git-contacts` to get a list of reviewers you should
>> +include in the CC list.
>> +
>
> Should we mention that the script can be passed to git-send-email(1) via
> `--cc-cmd=`?

Ack, will do. I think I can just copy/paste the existing guidance from
git-contact.txt which has this example:

    git send-email --cc-cmd='git contacts' feature/*.patch
Linus Arver April 4, 2024, 8:01 p.m. UTC | #8
Matthias Aßhauer <mha1993@live.de> writes:

> On Tue, 2 Apr 2024, Linus Arver via GitGitGadget wrote:
>
>> From: Linus Arver <linusa@google.com>
>>
>> Although we've had this script since 4d06402b1b (contrib: add
>> git-contacts helper, 2013-07-21), we don't mention it in our
>> introductory docs. Do so now.
>>
>> Signed-off-by: Linus Arver <linusa@google.com>
>> ---
>>    docs: recommend using contrib/contacts/git-contacts
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1704%2Flistx%2Freviewers-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1704/listx/reviewers-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/1704
>>
>> Documentation/MyFirstContribution.txt | 3 +++
>> Documentation/SubmittingPatches       | 4 ++++
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
>> index f06563e9817..eb1e27a82df 100644
>> --- a/Documentation/MyFirstContribution.txt
>> +++ b/Documentation/MyFirstContribution.txt
>> @@ -1116,6 +1116,9 @@ $ git send-email --to=target@example.com psuh/*.patch
>> NOTE: Check `git help send-email` for some other options which you may find
>> valuable, such as changing the Reply-to address or adding more CC and BCC lines.
>>
>> +NOTE: Use `contrib/contacts/git-contacts` to get a list of reviewers you should
>> +include in the CC list.
>> +
>> NOTE: When you are sending a real patch, it will go to git@vger.kernel.org - but
>> please don't send your patchset from the tutorial to the real mailing list! For
>> now, you can send it to yourself, to make sure you understand how it will look.
>> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
>> index e734a3f0f17..52d11ff510b 100644
>> --- a/Documentation/SubmittingPatches
>> +++ b/Documentation/SubmittingPatches
>> @@ -459,6 +459,10 @@ an explanation of changes between each iteration can be kept in
>> Git-notes and inserted automatically following the three-dash
>> line via `git format-patch --notes`.
>>
>> +[[suggested-reviewers]]
>> +Use `contrib/contacts/git-contacts` to get a list of reviewers you should
>> +include in the CC list.
>> +
>
> There is already a paragraph about this in Documentation/SubmittingPatches 
> just a few paragraphs below.
>
>> Send your patch with "To:" set to the mailing list, with "cc:" listing
>> people who are involved in the area you are touching (the `git
>> contacts` command in `contrib/contacts/` can help to
>> identify them), to solicit comments and reviews.  Also, when you made
>> trial merges of your topic to `next` and `seen`, you may have noticed
>> work by others conflicting with your changes.  There is a good possibility
>> that these people may know the area you are touching well.
>
> Could we improve the existing paragraph instead of duplicating this 
> information?

Ah, yes of course (somehow I missed that existing guidance). Will update.
diff mbox series

Patch

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index f06563e9817..eb1e27a82df 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -1116,6 +1116,9 @@  $ git send-email --to=target@example.com psuh/*.patch
 NOTE: Check `git help send-email` for some other options which you may find
 valuable, such as changing the Reply-to address or adding more CC and BCC lines.
 
+NOTE: Use `contrib/contacts/git-contacts` to get a list of reviewers you should
+include in the CC list.
+
 NOTE: When you are sending a real patch, it will go to git@vger.kernel.org - but
 please don't send your patchset from the tutorial to the real mailing list! For
 now, you can send it to yourself, to make sure you understand how it will look.
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index e734a3f0f17..52d11ff510b 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -459,6 +459,10 @@  an explanation of changes between each iteration can be kept in
 Git-notes and inserted automatically following the three-dash
 line via `git format-patch --notes`.
 
+[[suggested-reviewers]]
+Use `contrib/contacts/git-contacts` to get a list of reviewers you should
+include in the CC list.
+
 [[attachment]]
 Do not attach the patch as a MIME attachment, compressed or not.
 Do not let your e-mail client send quoted-printable.  Do not let