2021-10-07 08:43:34

by Thorsten Leemhuis

[permalink] [raw]
Subject: [PATCH v1 0/2] Prefer lore.kernel.org and explain Link: tags better

Lo! The regression tracking bot I'm working on can automatically mark an
entry as resolved, if the commit message of the fix uses a 'Link' tag to
the report. Many developers already place them, but it afaics would
improve matters to make this more explicit. Especially as I had missed
the modified section myself at first, as I simply grepped for 'Link:'
and only found an explanation in configure-git.rst.

Konstantin after posting v1 suggested to use lore.kernel.org instead or
lkml.kernel.org, which made me add a patch to realize this everywhere in
the docs.

v2:
- slightly reword after suggestiones from Konstantin (thx!)
- make this a patch series with an preparatory patch that does
s!lkml.kernel.org!lore.kernel.org! everywhere in the docs

v1: https://lore.kernel.org/r/7dff33afec555fed0bf033c910ca59f9f19f22f1.1633537634.git.linux@leemhuis.info/
- initial version

Ciao, Thorsten

Thorsten Leemhuis (2):
docs: use the lore redirector everywhere
docs: submitting-patches: make section about the Link: tag more
explicit

Documentation/asm-annotations.rst | 2 +-
.../kbuild/Kconfig.recursion-issue-02 | 2 +-
Documentation/maintainer/pull-requests.rst | 2 +-
Documentation/networking/msg_zerocopy.rst | 2 +-
Documentation/process/maintainer-tip.rst | 4 +--
Documentation/process/submitting-patches.rst | 34 ++++++++++++-------
.../it_IT/process/submitting-patches.rst | 4 +--
.../zh_CN/maintainer/pull-requests.rst | 2 +-
.../zh_CN/process/submitting-patches.rst | 4 +--
.../zh_TW/process/submitting-patches.rst | 4 +--
Documentation/x86/entry_64.rst | 2 +-
Documentation/x86/orc-unwinder.rst | 4 +--
12 files changed, 38 insertions(+), 28 deletions(-)


base-commit: b19511926cb50d59c57189739d03c21df325710f
--
2.31.1


2021-10-07 08:45:28

by Thorsten Leemhuis

[permalink] [raw]
Subject: [PATCH v1 2/2] docs: submitting-patches: make section about the Link: tag more explicit

Mention the 'Link' tag in the section about adding URLs to the commit
msg, which makes it easier to find its meaning with a text search. For
the same reason and to also improve comprehensibility provide an
example.

Slightly improve the text at the same time to make it more obvious
developers are meant to add links to issue reports in mailing list
archives, as those allow regression tracking efforts to automatically
check which bugs got resolved.

Move the section also downwards slightly, to reduce jumping back and
forth between aspects relevant for the top and the bottom part of the
commit msg.

CC: Konstantin Ryabitsev <[email protected]>
Signed-off-by: Thorsten Leemhuis <[email protected]>
---
Documentation/process/submitting-patches.rst | 32 +++++++++++++-------
1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index b0f31aa82fcd..8ba69332322f 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -96,17 +96,6 @@ instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour.

-If the patch fixes a logged bug entry, refer to that bug entry by
-number and URL. If the patch follows from a mailing list discussion,
-give a URL to the mailing list archive; use the https://lore.kernel.org/
-redirector with a ``Message-Id``, to ensure that the links cannot become
-stale.
-
-However, try to make your explanation understandable without external
-resources. In addition to giving a URL to a mailing list archive or
-bug, summarize the relevant points of the discussion that led to the
-patch as submitted.
-
If you want to refer to a specific commit, don't just refer to the
SHA-1 ID of the commit. Please also include the oneline summary of
the commit, to make it easier for reviewers to know what it is about.
@@ -123,6 +112,27 @@ collisions with shorter IDs a real possibility. Bear in mind that, even if
there is no collision with your six-character ID now, that condition may
change five years from now.

+Add 'Link:' tags with URLs pointing to related discussions and rationale
+behind the change whenever that makes sense. If your patch for example
+fixes a bug, add a tag with a URL referencing the report in the mailing
+list archives or a bug tracker; if the patch was discussed on a mailing
+list or originated in some discussion, point to it.
+
+When linking to mailing list archives, preferably use the lore.kernel.org
+message archiver service. To create the link URL, use the contents of the
+``Message-Id`` header of the message without the surrounding angle brackets.
+For example::
+
+ Link: https://lore.kernel.org/r/[email protected]
+
+Please check the link to make sure that it is actually working and points
+to the relevant message.
+
+However, try to make your explanation understandable without external
+resources. In addition to giving a URL to a mailing list archive or
+bug, summarize the relevant points of the discussion that led to the
+patch as submitted.
+
If your patch fixes a bug in a specific commit, e.g. you found an issue using
``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
the SHA-1 ID, and the one line summary. Do not split the tag across multiple
--
2.31.1

2021-10-07 09:33:08

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] docs: submitting-patches: make section about the Link: tag more explicit

On Thu, 07 Oct 2021, Thorsten Leemhuis <[email protected]> wrote:
> Mention the 'Link' tag in the section about adding URLs to the commit
> msg, which makes it easier to find its meaning with a text search. For
> the same reason and to also improve comprehensibility provide an
> example.
>
> Slightly improve the text at the same time to make it more obvious
> developers are meant to add links to issue reports in mailing list
> archives, as those allow regression tracking efforts to automatically
> check which bugs got resolved.
>
> Move the section also downwards slightly, to reduce jumping back and
> forth between aspects relevant for the top and the bottom part of the
> commit msg.

FWIW, we've been using the Link: tag in the drm-misc and drm-intel trees
to reference the patch (that became the commit) in the freedesktop.org
patchwork instance by message-id. This is almost exclusively the only
way we use the Link: tag, and we've been doing this for about 5 years
now. It gets automatically added by the tooling we use while applying
patches. You get to the discussion, patch series, and Intel CI test
results via the link.

For ages, References: tag has been used the way described in this patch.

BR,
Jani.

>
> CC: Konstantin Ryabitsev <[email protected]>
> Signed-off-by: Thorsten Leemhuis <[email protected]>
> ---
> Documentation/process/submitting-patches.rst | 32 +++++++++++++-------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index b0f31aa82fcd..8ba69332322f 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -96,17 +96,6 @@ instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
> to do frotz", as if you are giving orders to the codebase to change
> its behaviour.
>
> -If the patch fixes a logged bug entry, refer to that bug entry by
> -number and URL. If the patch follows from a mailing list discussion,
> -give a URL to the mailing list archive; use the https://lore.kernel.org/
> -redirector with a ``Message-Id``, to ensure that the links cannot become
> -stale.
> -
> -However, try to make your explanation understandable without external
> -resources. In addition to giving a URL to a mailing list archive or
> -bug, summarize the relevant points of the discussion that led to the
> -patch as submitted.
> -
> If you want to refer to a specific commit, don't just refer to the
> SHA-1 ID of the commit. Please also include the oneline summary of
> the commit, to make it easier for reviewers to know what it is about.
> @@ -123,6 +112,27 @@ collisions with shorter IDs a real possibility. Bear in mind that, even if
> there is no collision with your six-character ID now, that condition may
> change five years from now.
>
> +Add 'Link:' tags with URLs pointing to related discussions and rationale
> +behind the change whenever that makes sense. If your patch for example
> +fixes a bug, add a tag with a URL referencing the report in the mailing
> +list archives or a bug tracker; if the patch was discussed on a mailing
> +list or originated in some discussion, point to it.
> +
> +When linking to mailing list archives, preferably use the lore.kernel.org
> +message archiver service. To create the link URL, use the contents of the
> +``Message-Id`` header of the message without the surrounding angle brackets.
> +For example::
> +
> + Link: https://lore.kernel.org/r/[email protected]
> +
> +Please check the link to make sure that it is actually working and points
> +to the relevant message.
> +
> +However, try to make your explanation understandable without external
> +resources. In addition to giving a URL to a mailing list archive or
> +bug, summarize the relevant points of the discussion that led to the
> +patch as submitted.
> +
> If your patch fixes a bug in a specific commit, e.g. you found an issue using
> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> the SHA-1 ID, and the one line summary. Do not split the tag across multiple

--
Jani Nikula, Intel Open Source Graphics Center

2021-10-07 13:57:17

by Konstantin Ryabitsev

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Prefer lore.kernel.org and explain Link: tags better

On Thu, Oct 07, 2021 at 10:04:59AM +0200, Thorsten Leemhuis wrote:
> v2:
> - slightly reword after suggestiones from Konstantin (thx!)
> - make this a patch series with an preparatory patch that does
> s!lkml.kernel.org!lore.kernel.org! everywhere in the docs

Reviewed-by: Konstantin Ryabitsev <[email protected]>

Thanks,
-K

2021-10-08 09:49:32

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] docs: submitting-patches: make section about the Link: tag more explicit

(sorry, sending it a second time, vger rejected it earlier - and I have
an idea now what went wrong)

On 07.10.21 11:31, Jani Nikula wrote:
> On Thu, 07 Oct 2021, Thorsten Leemhuis <[email protected]> wrote:
>> Mention the 'Link' tag in the section about adding URLs to the commit
>> msg, which makes it easier to find its meaning with a text search. For
>> the same reason and to also improve comprehensibility provide an
>> example.
>>
>> Slightly improve the text at the same time to make it more obvious
>> developers are meant to add links to issue reports in mailing list
>> archives, as those allow regression tracking efforts to automatically
>> check which bugs got resolved.
>>
>> Move the section also downwards slightly, to reduce jumping back and
>> forth between aspects relevant for the top and the bottom part of the
>> commit msg.
>
> FWIW, we've been using the Link: tag in the drm-misc and drm-intel trees
> to reference the patch (that became the commit) in the freedesktop.org
> patchwork instance by message-id. This is almost exclusively the only
> way we use the Link: tag, and we've been doing this for about 5 years
> now. [...]

Which afaik is totally appropriate and the way it is used most of the
time, especially since more and more maintainers use b4.

But it afaics also gets used to refer to bug reports:

$ git log v5.14.. | grep ' Link: https://bugzilla.kernel.org/' | wc -
8

But maybe that's not the way it is intended.

> For ages, References: tag has been used the way described in this patch.

Hmmm, seems other developers/subsystems handle that tag a bit different
as well. I simply looked for "References:" in v5.14.. (excluding drm)
and for example found the following in
https://git.kernel.org/torvalds/c/19532869feb9b0a97d17ddc14609d1e53a5b60db

```
> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> Link: https://lkml.kernel.org/r/[email protected]
```

This commit uses "References:" in a similar way:
https://git.kernel.org/torvalds/c/13be2efc390acd2a46a69a359f6efc00ca434599

Maybe it's time to generate a table with the "official tags" (and create
separate tags for the different purposes at the same time as well?).
Wasn't something like that a topic in the past? My mind vaguely recalls
a lwn.net article about tags and their misuse, but I couldn't find it.
And maybe my mind is mixing things up anyway and remembers something
that never happened. :-/

Ciao, Thorsten


>> CC: Konstantin Ryabitsev <[email protected]>
>> Signed-off-by: Thorsten Leemhuis <[email protected]>
>> ---
>> Documentation/process/submitting-patches.rst | 32 +++++++++++++-------
>> 1 file changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> index b0f31aa82fcd..8ba69332322f 100644
>> --- a/Documentation/process/submitting-patches.rst
>> +++ b/Documentation/process/submitting-patches.rst
>> @@ -96,17 +96,6 @@ instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
>> to do frotz", as if you are giving orders to the codebase to change
>> its behaviour.
>>
>> -If the patch fixes a logged bug entry, refer to that bug entry by
>> -number and URL. If the patch follows from a mailing list discussion,
>> -give a URL to the mailing list archive; use the https://lore.kernel.org/
>> -redirector with a ``Message-Id``, to ensure that the links cannot become
>> -stale.
>> -
>> -However, try to make your explanation understandable without external
>> -resources. In addition to giving a URL to a mailing list archive or
>> -bug, summarize the relevant points of the discussion that led to the
>> -patch as submitted.
>> -
>> If you want to refer to a specific commit, don't just refer to the
>> SHA-1 ID of the commit. Please also include the oneline summary of
>> the commit, to make it easier for reviewers to know what it is about.
>> @@ -123,6 +112,27 @@ collisions with shorter IDs a real possibility. Bear in mind that, even if
>> there is no collision with your six-character ID now, that condition may
>> change five years from now.
>>
>> +Add 'Link:' tags with URLs pointing to related discussions and rationale
>> +behind the change whenever that makes sense. If your patch for example
>> +fixes a bug, add a tag with a URL referencing the report in the mailing
>> +list archives or a bug tracker; if the patch was discussed on a mailing
>> +list or originated in some discussion, point to it.
>> +
>> +When linking to mailing list archives, preferably use the lore.kernel.org
>> +message archiver service. To create the link URL, use the contents of the
>> +``Message-Id`` header of the message without the surrounding angle brackets.
>> +For example::
>> +
>> + Link: https://lore.kernel.org/r/[email protected]
>> +
>> +Please check the link to make sure that it is actually working and points
>> +to the relevant message.
>> +
>> +However, try to make your explanation understandable without external
>> +resources. In addition to giving a URL to a mailing list archive or
>> +bug, summarize the relevant points of the discussion that led to the
>> +patch as submitted.
>> +
>> If your patch fixes a bug in a specific commit, e.g. you found an issue using
>> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
>> the SHA-1 ID, and the one line summary. Do not split the tag across multiple
>

2021-10-12 20:06:03

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Prefer lore.kernel.org and explain Link: tags better

Thorsten Leemhuis <[email protected]> writes:

> Lo! The regression tracking bot I'm working on can automatically mark an
> entry as resolved, if the commit message of the fix uses a 'Link' tag to
> the report. Many developers already place them, but it afaics would
> improve matters to make this more explicit. Especially as I had missed
> the modified section myself at first, as I simply grepped for 'Link:'
> and only found an explanation in configure-git.rst.
>
> Konstantin after posting v1 suggested to use lore.kernel.org instead or
> lkml.kernel.org, which made me add a patch to realize this everywhere in
> the docs.
>
> v2:
> - slightly reword after suggestiones from Konstantin (thx!)
> - make this a patch series with an preparatory patch that does
> s!lkml.kernel.org!lore.kernel.org! everywhere in the docs
>
> v1: https://lore.kernel.org/r/7dff33afec555fed0bf033c910ca59f9f19f22f1.1633537634.git.linux@leemhuis.info/
> - initial version
>
> Ciao, Thorsten
>
> Thorsten Leemhuis (2):
> docs: use the lore redirector everywhere

OK, I've applied this one, thanks.

> docs: submitting-patches: make section about the Link: tag more
> explicit

There was a comment on this one, so I've not (yet) applied it.

FWIW, I, too, have the Link: tags put in automatically when I apply a
patch, as Jani described; it's a simple hook in
.git/hooks/applypatch-msg. That seems worth mentioning here more than
instructions on how to construct the link - I doubt many people do it
manually.

Thanks,

jon

2021-10-13 04:40:18

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Prefer lore.kernel.org and explain Link: tags better

[sorry, once again I have to sent it a second time, as vger rejected it;
thunderbird once again messed for some reason used
"Content-Transfer-Encoding: 7bit" then it should have used 8bit; I tried
to track this down by sending mails to myself, but then it sets 8bit
correctly when needed :-/ ]

On 12.10.21 22:03, Jonathan Corbet wrote:
> Thorsten Leemhuis <[email protected]> writes:
>
>> Lo! The regression tracking bot I'm working on can automatically mark an
>> entry as resolved, if the commit message of the fix uses a 'Link' tag to
>> the report. Many developers already place them, but it afaics would
>> improve matters to make this more explicit. Especially as I had missed
>> the modified section myself at first, as I simply grepped for 'Link:'
>> and only found an explanation in configure-git.rst.
>>
>> Konstantin after posting v1 suggested to use lore.kernel.org instead or
>> lkml.kernel.org, which made me add a patch to realize this everywhere in
>> the docs.
>>
>> v2:
>> - slightly reword after suggestiones from Konstantin (thx!)
>> - make this a patch series with an preparatory patch that does
>> s!lkml.kernel.org!lore.kernel.org! everywhere in the docs
>>
>> v1: https://lore.kernel.org/r/7dff33afec555fed0bf033c910ca59f9f19f22f1.1633537634.git.linux@leemhuis.info/
>> - initial version
>>
>> Ciao, Thorsten
>>
>> Thorsten Leemhuis (2):
>> docs: use the lore redirector everywhere
>
> OK, I've applied this one, thanks.

Thx!

>> docs: submitting-patches: make section about the Link: tag more
>> explicit

Yeah, totally fine.

> There was a comment on this one, so I've not (yet) applied it.
>
> FWIW, I, too, have the Link: tags put in automatically when I apply a
> patch, as Jani described; it's a simple hook in
> .git/hooks/applypatch-msg. That seems worth mentioning here more than
> instructions on how to construct the link - I doubt many people do it
> manually.

Well, that is already explained in
Documentation/maintainer/configure-git.rst afaics -- which is the better
place afaics, as that is something maintainers should do, and not
something people should do when submitting a patch.

Ciao, Thorsten

2021-10-21 08:47:18

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] docs: submitting-patches: make section about the Link: tag more explicit

On 08.10.21 11:47, Thorsten Leemhuis wrote:
> On 07.10.21 11:31, Jani Nikula wrote:
>> On Thu, 07 Oct 2021, Thorsten Leemhuis <[email protected]> wrote:
>>> Mention the 'Link' tag in the section about adding URLs to the commit
>>> msg, which makes it easier to find its meaning with a text search. For
>>> the same reason and to also improve comprehensibility provide an
>>> example.
>>>
>>> Slightly improve the text at the same time to make it more obvious
>>> developers are meant to add links to issue reports in mailing list
>>> archives, as those allow regression tracking efforts to automatically
>>> check which bugs got resolved.
>>>
>>> Move the section also downwards slightly, to reduce jumping back and
>>> forth between aspects relevant for the top and the bottom part of the
>>> commit msg.
>>
>> FWIW, we've been using the Link: tag in the drm-misc and drm-intel trees
>> to reference the patch (that became the commit) in the freedesktop.org
>> patchwork instance by message-id. This is almost exclusively the only
>> way we use the Link: tag, and we've been doing this for about 5 years
>> now. [...]
>
> Which afaik is totally appropriate and the way it is used most of the
> time, especially since more and more maintainers use b4.
>
> But it afaics also gets used to refer to bug reports:
>
> $ git log v5.14.. | grep ' Link: https://bugzilla.kernel.org/' | wc -
> 8
>
> But maybe that's not the way it is intended.
>
>> For ages, References: tag has been used the way described in this patch.
>
> Hmmm, seems other developers/subsystems handle that tag a bit different
> as well. I simply looked for "References:" in v5.14.. (excluding drm)
> and for example found the following in
> https://git.kernel.org/torvalds/c/19532869feb9b0a97d17ddc14609d1e53a5b60db
>
> ```
>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>> Link: https://lkml.kernel.org/r/[email protected]
> ```
>
> This commit uses "References:" in a similar way:
> https://git.kernel.org/torvalds/c/13be2efc390acd2a46a69a359f6efc00ca434599

FWIW, Linus today coincidentally in
https://lore.kernel.org/lkml/CAHk-=wgBhyLhQLPem1vybKNt7BKP+=qF=veBgc7VirZaXn4FUw@mail.gmail.com/
wrote:

```
So _primarily_ the "Link:" line should be about background - and for
"oh, there was discussion about this patch after it was committed".
```

I'd consider a bug/regression report as "background", so from my point
of view I'd say what my patch does is correct. Or do you disagree Jani?
If not I'd like to move on and ask Jonathan to merge it.

FWIW, I'll...

> Maybe it's time to generate a table with the "official tags" (and create
> separate tags for the different purposes at the same time as well?).

...keep this in mind to maybe get back to it sooner or later.

Ciao, Thorsten

> Wasn't something like that a topic in the past? My mind vaguely recalls
> a lwn.net article about tags and their misuse, but I couldn't find it.
> And maybe my mind is mixing things up anyway and remembers something
> that never happened. :-/
>
> Ciao, Thorsten
>
>
>>> CC: Konstantin Ryabitsev <[email protected]>
>>> Signed-off-by: Thorsten Leemhuis <[email protected]>
>>> ---
>>> Documentation/process/submitting-patches.rst | 32 +++++++++++++-------
>>> 1 file changed, 21 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>>> index b0f31aa82fcd..8ba69332322f 100644
>>> --- a/Documentation/process/submitting-patches.rst
>>> +++ b/Documentation/process/submitting-patches.rst
>>> @@ -96,17 +96,6 @@ instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
>>> to do frotz", as if you are giving orders to the codebase to change
>>> its behaviour.
>>>
>>> -If the patch fixes a logged bug entry, refer to that bug entry by
>>> -number and URL. If the patch follows from a mailing list discussion,
>>> -give a URL to the mailing list archive; use the https://lore.kernel.org/
>>> -redirector with a ``Message-Id``, to ensure that the links cannot become
>>> -stale.
>>> -
>>> -However, try to make your explanation understandable without external
>>> -resources. In addition to giving a URL to a mailing list archive or
>>> -bug, summarize the relevant points of the discussion that led to the
>>> -patch as submitted.
>>> -
>>> If you want to refer to a specific commit, don't just refer to the
>>> SHA-1 ID of the commit. Please also include the oneline summary of
>>> the commit, to make it easier for reviewers to know what it is about.
>>> @@ -123,6 +112,27 @@ collisions with shorter IDs a real possibility. Bear in mind that, even if
>>> there is no collision with your six-character ID now, that condition may
>>> change five years from now.
>>>
>>> +Add 'Link:' tags with URLs pointing to related discussions and rationale
>>> +behind the change whenever that makes sense. If your patch for example
>>> +fixes a bug, add a tag with a URL referencing the report in the mailing
>>> +list archives or a bug tracker; if the patch was discussed on a mailing
>>> +list or originated in some discussion, point to it.
>>> +
>>> +When linking to mailing list archives, preferably use the lore.kernel.org
>>> +message archiver service. To create the link URL, use the contents of the
>>> +``Message-Id`` header of the message without the surrounding angle brackets.
>>> +For example::
>>> +
>>> + Link: https://lore.kernel.org/r/[email protected]
>>> +
>>> +Please check the link to make sure that it is actually working and points
>>> +to the relevant message.
>>> +
>>> +However, try to make your explanation understandable without external
>>> +resources. In addition to giving a URL to a mailing list archive or
>>> +bug, summarize the relevant points of the discussion that led to the
>>> +patch as submitted.
>>> +
>>> If your patch fixes a bug in a specific commit, e.g. you found an issue using
>>> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
>>> the SHA-1 ID, and the one line summary. Do not split the tag across multiple
>>
>

2021-10-21 09:49:26

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] docs: submitting-patches: make section about the Link: tag more explicit

On Thu, 21 Oct 2021, Thorsten Leemhuis <[email protected]> wrote:
> On 08.10.21 11:47, Thorsten Leemhuis wrote:
>> On 07.10.21 11:31, Jani Nikula wrote:
>>> On Thu, 07 Oct 2021, Thorsten Leemhuis <[email protected]> wrote:
>>>> Mention the 'Link' tag in the section about adding URLs to the commit
>>>> msg, which makes it easier to find its meaning with a text search. For
>>>> the same reason and to also improve comprehensibility provide an
>>>> example.
>>>>
>>>> Slightly improve the text at the same time to make it more obvious
>>>> developers are meant to add links to issue reports in mailing list
>>>> archives, as those allow regression tracking efforts to automatically
>>>> check which bugs got resolved.
>>>>
>>>> Move the section also downwards slightly, to reduce jumping back and
>>>> forth between aspects relevant for the top and the bottom part of the
>>>> commit msg.
>>>
>>> FWIW, we've been using the Link: tag in the drm-misc and drm-intel trees
>>> to reference the patch (that became the commit) in the freedesktop.org
>>> patchwork instance by message-id. This is almost exclusively the only
>>> way we use the Link: tag, and we've been doing this for about 5 years
>>> now. [...]
>>
>> Which afaik is totally appropriate and the way it is used most of the
>> time, especially since more and more maintainers use b4.
>>
>> But it afaics also gets used to refer to bug reports:
>>
>> $ git log v5.14.. | grep ' Link: https://bugzilla.kernel.org/' | wc -
>> 8
>>
>> But maybe that's not the way it is intended.
>>
>>> For ages, References: tag has been used the way described in this patch.
>>
>> Hmmm, seems other developers/subsystems handle that tag a bit different
>> as well. I simply looked for "References:" in v5.14.. (excluding drm)
>> and for example found the following in
>> https://git.kernel.org/torvalds/c/19532869feb9b0a97d17ddc14609d1e53a5b60db
>>
>> ```
>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>>> Link: https://lkml.kernel.org/r/[email protected]
>> ```
>>
>> This commit uses "References:" in a similar way:
>> https://git.kernel.org/torvalds/c/13be2efc390acd2a46a69a359f6efc00ca434599
>
> FWIW, Linus today coincidentally in
> https://lore.kernel.org/lkml/CAHk-=wgBhyLhQLPem1vybKNt7BKP+=qF=veBgc7VirZaXn4FUw@mail.gmail.com/
> wrote:
>
> ```
> So _primarily_ the "Link:" line should be about background - and for
> "oh, there was discussion about this patch after it was committed".
> ```

Discussion about a patch *after* it was committed will likely happen in
the mailing list thread where the patch was posted. So kind of what we
do with Link.

> I'd consider a bug/regression report as "background", so from my point
> of view I'd say what my patch does is correct. Or do you disagree Jani?
> If not I'd like to move on and ask Jonathan to merge it.

Fair enough.

I think the main thing to accept is that while we may add kernel wide
guidance, we'll end up with different subsystems and drivers having
varying conventions no matter what.

> FWIW, I'll...
>
>> Maybe it's time to generate a table with the "official tags" (and create
>> separate tags for the different purposes at the same time as well?).
>
> ...keep this in mind to maybe get back to it sooner or later.

That is going to be one massive bikeshed fest. :|

BR,
Jani.

>
> Ciao, Thorsten
>
>> Wasn't something like that a topic in the past? My mind vaguely recalls
>> a lwn.net article about tags and their misuse, but I couldn't find it.
>> And maybe my mind is mixing things up anyway and remembers something
>> that never happened. :-/
>>
>> Ciao, Thorsten
>>
>>
>>>> CC: Konstantin Ryabitsev <[email protected]>
>>>> Signed-off-by: Thorsten Leemhuis <[email protected]>
>>>> ---
>>>> Documentation/process/submitting-patches.rst | 32 +++++++++++++-------
>>>> 1 file changed, 21 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>>>> index b0f31aa82fcd..8ba69332322f 100644
>>>> --- a/Documentation/process/submitting-patches.rst
>>>> +++ b/Documentation/process/submitting-patches.rst
>>>> @@ -96,17 +96,6 @@ instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
>>>> to do frotz", as if you are giving orders to the codebase to change
>>>> its behaviour.
>>>>
>>>> -If the patch fixes a logged bug entry, refer to that bug entry by
>>>> -number and URL. If the patch follows from a mailing list discussion,
>>>> -give a URL to the mailing list archive; use the https://lore.kernel.org/
>>>> -redirector with a ``Message-Id``, to ensure that the links cannot become
>>>> -stale.
>>>> -
>>>> -However, try to make your explanation understandable without external
>>>> -resources. In addition to giving a URL to a mailing list archive or
>>>> -bug, summarize the relevant points of the discussion that led to the
>>>> -patch as submitted.
>>>> -
>>>> If you want to refer to a specific commit, don't just refer to the
>>>> SHA-1 ID of the commit. Please also include the oneline summary of
>>>> the commit, to make it easier for reviewers to know what it is about.
>>>> @@ -123,6 +112,27 @@ collisions with shorter IDs a real possibility. Bear in mind that, even if
>>>> there is no collision with your six-character ID now, that condition may
>>>> change five years from now.
>>>>
>>>> +Add 'Link:' tags with URLs pointing to related discussions and rationale
>>>> +behind the change whenever that makes sense. If your patch for example
>>>> +fixes a bug, add a tag with a URL referencing the report in the mailing
>>>> +list archives or a bug tracker; if the patch was discussed on a mailing
>>>> +list or originated in some discussion, point to it.
>>>> +
>>>> +When linking to mailing list archives, preferably use the lore.kernel.org
>>>> +message archiver service. To create the link URL, use the contents of the
>>>> +``Message-Id`` header of the message without the surrounding angle brackets.
>>>> +For example::
>>>> +
>>>> + Link: https://lore.kernel.org/r/[email protected]
>>>> +
>>>> +Please check the link to make sure that it is actually working and points
>>>> +to the relevant message.
>>>> +
>>>> +However, try to make your explanation understandable without external
>>>> +resources. In addition to giving a URL to a mailing list archive or
>>>> +bug, summarize the relevant points of the discussion that led to the
>>>> +patch as submitted.
>>>> +
>>>> If your patch fixes a bug in a specific commit, e.g. you found an issue using
>>>> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
>>>> the SHA-1 ID, and the one line summary. Do not split the tag across multiple
>>>
>>

--
Jani Nikula, Intel Open Source Graphics Center