2022-12-06 01:24:55

by Joe Perches

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote:
> Apparently MAINTAINERS is hard to read. Please review?
>
> Thanks.
>
> Begin forwarded message:
>
> Date: Sun, 4 Dec 2022 12:33:37 +0100
> From: Kai Wasserb?ch <[email protected]>
> To: [email protected]
> Cc: Andrew Morton <[email protected]>, Thorsten Leemhuis <[email protected]>
> Subject: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:
>
>
> Hey,
> Thorsten Leemhuis suggested the following two changes to checkpatch, which
> I hereby humbly submit for review. Please let me know if any changes
> would be required for acceptance.
>
> NOTES:
> - checkpatch is rather long, so I might have chosen the wrong section to
> add these two checks. Any better placement suggestion is welcome.
> - checkpatch implements custum versions of Perl core modules, that might
> be an future area for clean-ups? Eg. right off the bat there is a
> `uniq` implementation. List::Util (core module since 5.8.0 (5.7.3 to
> be pedantic)) has a far better performing version in XS.

Maybe. I don't know there are that many generic functions
that could be used. You are welcome find some.

>
> Cheers,
> Kai
>
> Suggested-by: Thorsten Leemhuis <[email protected]>
> Signed-off-by: Kai Wasserb?ch <[email protected]>
>
>
> Kai Wasserb?ch (2):
> feat: checkpatch: error on usage of a Buglink tag in the commit log

Why, what's wrong with a buglink reference?

> feat: checkpatch: Warn about Reported-by: not being followed by a
> Link:

I think this is unnecessary.


2022-12-06 01:56:14

by Joe Perches

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

On Mon, 2022-12-05 at 17:02 -0800, Joe Perches wrote:
> On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote:
> > Date: Sun, 4 Dec 2022 12:33:37 +0100
> > From: Kai Wasserb?ch <[email protected]>
[]
> > - checkpatch implements custum versions of Perl core modules, that might
> > be an future area for clean-ups? Eg. right off the bat there is a
> > `uniq` implementation. List::Util (core module since 5.8.0 (5.7.3 to
> > be pedantic)) has a far better performing version in XS.

> Maybe. I don't know there are that many generic functions
> that could be used. You are welcome find some.

uniq isn't used in checkpatch since

commit 52178ce01335d9d76611c3a5198b8778cb9b03f5
Author: Dwaipayan Ray <[email protected]>
Date: Fri Feb 26 15:08:26 2021 +0530

checkpatch: add verbose mode

and could be removed.

2022-12-06 05:24:53

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

Hi Joe! Many thx for looking into this.

On 06.12.22 02:02, Joe Perches wrote:
> On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote:
>> Begin forwarded message:
>>
>> Date: Sun, 4 Dec 2022 12:33:37 +0100
>> From: Kai Wasserbäch <[email protected]>
>> To: [email protected]
>> Cc: Andrew Morton <[email protected]>, Thorsten Leemhuis <[email protected]>
>> Subject: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:
>>
>> Thorsten Leemhuis suggested the following two changes to checkpatch, which
>> I hereby humbly submit for review. Please let me know if any changes
>> would be required for acceptance.
> [...]
>> Suggested-by: Thorsten Leemhuis <[email protected]>
>> Signed-off-by: Kai Wasserbäch <[email protected]>
>>
>> Kai Wasserbäch (2):
>> feat: checkpatch: error on usage of a Buglink tag in the commit log
>
> Why, what's wrong with a buglink reference?

Long story short: Linus doesn't like it:

```
>> BugLink: https://lore.kernel.org/r/20220610205038.GA3050413@paulmck-ThinkPad-P17-Gen-1
>> BugLink: https://lore.kernel.org/r/CAMdYzYpF4FNTBPZsEFeWRuEwSies36QM_As8osPWZSr2q-viEA@mail.gmail.com
> [...]
> please stop making up random tags that make no sense.
>
> Just use "Link:"
> [...]
>
> It's extra context to the commit, in case somebody wants to know the
> history. The "bug" part is (and always should be) already explained in
> the commit message, there's absolutely no point in adding soem extra
> noise to the "Link:" tag.>
```
That's a quote from
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/

In that message he also links to another mail from him, where he wrote:
```
> I think "BugLink:" is silly, because that's just a regular link.
> What's the upside of saying "Bug" in there? If you're fixing a bug,
> and are linking to reports and discussions by people, what does that
> "bug" buy you apart from another ugly CamelCase thing?
>
> In other words, in "BugLink:", neither "Bug" nor "Link" is actually meaningful.
>
> The current "Link:" tag exists AS A GENERIC WAY TO SAY "look, here's
> more information". That's the point. The word "Link" has no other
> meaning, and trying to then combine it with other things is worthless.
```
https://lore.kernel.org/all/CAHk-=wgzRUT1fBpuz3xcN+YdsX0SxqOzHWRtj0ReHpUBb5TKbA@mail.gmail.com/

Our docs say to use Link in this case, too; see
Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)

And using various tags for the same thing makes it also harder for
external scripts that look at commits to take further action -- like the
regression tracking bot I write and use for my work.

All of that obviously should have been in patch description. Let me
resubmit that patch with all of that in there, or are you dead against
this idea now?

>> feat: checkpatch: Warn about Reported-by: not being followed by a
>> Link:
>
> I think this is unnecessary.

I expect this to cause more discussion, which is why this should be in a
separate submission. But in the end the reasons are similar as above:
(1) Linus really want to see those links to make things easier for
future code archeologists. (2) My regression tracking efforts heavily
rely on those Links, as they allow to automatically connect reports with
patches and commits to fix the reported regression; without those the
tracking is a PITA and doesn't scale.

Together that IMHO is strong enough reason to warn in this case.

Two quotes from Linus to show that he really wants those links:

```
> The current "Link:" tag exists AS A GENERIC WAY TO SAY "look, here's
> more information". That's the point. The word "Link" has no other
> meaning, and trying to then combine it with other things is worthless.
>
> And a bug report is exactly that kind of "look, here's more
> information".
>
> [...]>
> The commit message should have enough of an explanation on its own
> ("Reported-by:" and the regular "Link:" to the report) that there's
> *no* excuse to try to make a bug report link special.
```
https://lore.kernel.org/all/CAHk-=wgzRUT1fBpuz3xcN+YdsX0SxqOzHWRtj0ReHpUBb5TKbA@mail.gmail.com/

```
>> The flag was dropped because it was causing drivers that requested their
>> memory resource with pci_request_region() to fail with -EBUSY (e.g: the
>> vmwgfx driver):
>>
>> https://www.spinics.net/lists/dri-devel/msg329672.html
>
> See, *that* link would have been useful in the commit.
>
> Again, the commit has a link to the patch *submission*, which is
> almost entirely useless. There's no link to the actual problem the
> patch fixes.
>> [...]>
> I _really_ wish the -tip tree had more links to the actual problem
> reports and explanations, rather than links to the patch submission.
>
> [...]>
> Put another way: I can see that
>
> Reported-by: Zhangfei Gao <[email protected]>
>
> in the commit, but I don't have a clue what the actual report was, and
> there really isn't enough information in the commit itself, except for
> a fairly handwavy "Device drivers might, for instance, still need to
> flush operations.."
```
https://lore.kernel.org/amd-gfx/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/

This also obviously would need to be in the patch description in some
form. I can take care of that.

Ciao, Thorsten

2022-12-06 05:56:39

by Joe Perches

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

On Tue, 2022-12-06 at 05:55 +0100, Thorsten Leemhuis wrote:
> Hi Joe! Many thx for looking into this.
>
> On 06.12.22 02:02, Joe Perches wrote:
> > On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote:
> > > Begin forwarded message:
> > >
> > > Date: Sun, 4 Dec 2022 12:33:37 +0100
> > > From: Kai Wasserb?ch <[email protected]>
> > > To: [email protected]
> > > Cc: Andrew Morton <[email protected]>, Thorsten Leemhuis <[email protected]>
> > > Subject: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:
> > >
> > > Thorsten Leemhuis suggested the following two changes to checkpatch, which
> > > I hereby humbly submit for review. Please let me know if any changes
> > > would be required for acceptance.
> > [...]
> > > Suggested-by: Thorsten Leemhuis <[email protected]>
> > > Signed-off-by: Kai Wasserb?ch <[email protected]>
> > >
> > > Kai Wasserb?ch (2):
> > > feat: checkpatch: error on usage of a Buglink tag in the commit log
> >
> > Why, what's wrong with a buglink reference?
>
> Long story short: Linus doesn't like it:
>
> ```
> > > BugLink: https://lore.kernel.org/r/20220610205038.GA3050413@paulmck-ThinkPad-P17-Gen-1
> > > BugLink: https://lore.kernel.org/r/CAMdYzYpF4FNTBPZsEFeWRuEwSies36QM_As8osPWZSr2q-viEA@mail.gmail.com
> > [...]
> > please stop making up random tags that make no sense.
> >
> > Just use "Link:"
> > [...]
> >
> > It's extra context to the commit, in case somebody wants to know the
> > history. The "bug" part is (and always should be) already explained in
> > the commit message, there's absolutely no point in adding soem extra
> > noise to the "Link:" tag.>
> ```
> That's a quote from
> https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
>
> In that message he also links to another mail from him, where he wrote:
> ```
> > I think "BugLink:" is silly, because that's just a regular link.
> > What's the upside of saying "Bug" in there? If you're fixing a bug,
> > and are linking to reports and discussions by people, what does that
> > "bug" buy you apart from another ugly CamelCase thing?
> >
> > In other words, in "BugLink:", neither "Bug" nor "Link" is actually meaningful.
> >
> > The current "Link:" tag exists AS A GENERIC WAY TO SAY "look, here's
> > more information". That's the point. The word "Link" has no other
> > meaning, and trying to then combine it with other things is worthless.
> ```
> https://lore.kernel.org/all/CAHk-=wgzRUT1fBpuz3xcN+YdsX0SxqOzHWRtj0ReHpUBb5TKbA@mail.gmail.com/
>
> Our docs say to use Link in this case, too; see
> Documentation/process/submitting-patches.rst
> (http://docs.kernel.org/process/submitting-patches.html) and
> Documentation/process/5.Posting.rst
> (https://docs.kernel.org/process/5.Posting.html)
>
> And using various tags for the same thing makes it also harder for
> external scripts that look at commits to take further action -- like the
> regression tracking bot I write and use for my work.
>
> All of that obviously should have been in patch description. Let me
> resubmit that patch with all of that in there, or are you dead against
> this idea now?

No, better commit description would be useful and perhaps a more
generic, "is the thing in front of a URI/URL" a known/supported entry,
instead of using an known invalid test would be a better mechanism.

> > > feat: checkpatch: Warn about Reported-by: not being followed by a
> > > Link:
> >
> > I think this is unnecessary.
>
> I expect this to cause more discussion, which is why this should be in a
> separate submission. But in the end the reasons are similar as above:
> (1) Linus really want to see those links to make things easier for
> future code archeologists. (2) My regression tracking efforts heavily
> rely on those Links, as they allow to automatically connect reports with
> patches and commits to fix the reported regression; without those the
> tracking is a PITA and doesn't scale.
>
> Together that IMHO is strong enough reason to warn in this case.

Maybe, I think there might be some value in not intermixing signature
tags and other tags though.

2022-12-06 07:07:47

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

On 06.12.22 06:54, Joe Perches wrote:
> On Tue, 2022-12-06 at 05:55 +0100, Thorsten Leemhuis wrote:
>> On 06.12.22 02:02, Joe Perches wrote:
>>> On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote:
>>>> Begin forwarded message:
>>>>
>>>> Date: Sun, 4 Dec 2022 12:33:37 +0100
>>>> From: Kai Wasserbäch <[email protected]>
>>>> To: [email protected]
>>>> Cc: Andrew Morton <[email protected]>, Thorsten Leemhuis <[email protected]>
>>>> Subject: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:
>>>>
>>>> Thorsten Leemhuis suggested the following two changes to checkpatch, which
>>>> I hereby humbly submit for review. Please let me know if any changes
>>>> would be required for acceptance.
>>> [...]
>>>> Suggested-by: Thorsten Leemhuis <[email protected]>
>>>> Signed-off-by: Kai Wasserbäch <[email protected]>
>>>>
>>>> Kai Wasserbäch (2):
>>>> feat: checkpatch: error on usage of a Buglink tag in the commit log
>>>
>>> Why, what's wrong with a buglink reference?
>>
>> Long story short: Linus doesn't like it:
>
> [...]
>
>> All of that obviously should have been in patch description. Let me
>> resubmit that patch with all of that in there, or are you dead against
>> this idea now?
>
> No, better commit description would be useful

I'll prepare something.

> and perhaps a more
> generic, "is the thing in front of a URI/URL" a known/supported entry,
> instead of using an known invalid test would be a better mechanism.

Are you sure about that? It's not that I disagree completely, but it
sounds overly restrictive to me and makes it harder for new tags to
evolve in case we might want them.

And what tags would be on this allow-list? Anything else then "Link" and
"Patchwork"? Those are the ones that looked common and valid to me when
I ran

git log --grep='http' v4.0.. | grep http | grep -v ' Link: ' | less

and skimmed the output. Maybe "Datasheet" should be allowed, too -- not
sure.

But I found a few others that likely should be on the disallow list:
"Closes:", "Bug:", "Gitlab issue:", "References:", "Ref:", "Bugzilla:",
"RHBZ:", and "link", as "Link" should be used instead in all of these
cases afaics.

>>>> feat: checkpatch: Warn about Reported-by: not being followed by a
>>>> Link:
>>>
>>> I think this is unnecessary.
>>
>> I expect this to cause more discussion, which is why this should be in a
>> separate submission. But in the end the reasons are similar as above:
>> (1) Linus really want to see those links to make things easier for
>> future code archeologists. (2) My regression tracking efforts heavily
>> rely on those Links, as they allow to automatically connect reports with
>> patches and commits to fix the reported regression; without those the
>> tracking is a PITA and doesn't scale.
>>
>> Together that IMHO is strong enough reason to warn in this case.
>
> Maybe, I think there might be some value in not intermixing signature
> tags and other tags though.

Hmm, sorry, not sure if I understood you here. Why do you consider
"Reported-by:" a signature tag? Isn't it more of an "appreciation" tag,
IOW, a kind of 'thank you' hat tip to the reporter?

Ciao, Thorsten

2022-12-06 07:45:18

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

On 06.12.22 07:27, Thorsten Leemhuis wrote:
> On 06.12.22 06:54, Joe Perches wrote:
>> On Tue, 2022-12-06 at 05:55 +0100, Thorsten Leemhuis wrote:
>>> On 06.12.22 02:02, Joe Perches wrote:
>>>> On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote:
>>>>> Begin forwarded message:
>>>>>
>>>>> Date: Sun, 4 Dec 2022 12:33:37 +0100
>>>>> From: Kai Wasserbäch <[email protected]>
>>>>> To: [email protected]
>>>>> Cc: Andrew Morton <[email protected]>, Thorsten Leemhuis <[email protected]>
>>>>> Subject: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:
>>>>>
>>>>> Thorsten Leemhuis suggested the following two changes to checkpatch, which
>>>>> I hereby humbly submit for review. Please let me know if any changes
>>>>> would be required for acceptance.
>>>> [...]
>>>>> Suggested-by: Thorsten Leemhuis <[email protected]>
>>>>> Signed-off-by: Kai Wasserbäch <[email protected]>
>>>>>
>>>>> Kai Wasserbäch (2):
>>>>> feat: checkpatch: error on usage of a Buglink tag in the commit log
>>>>
>>>> Why, what's wrong with a buglink reference?
>>>
>>> Long story short: Linus doesn't like it:
>>
>> [...]
>>
>>> All of that obviously should have been in patch description. Let me
>>> resubmit that patch with all of that in there, or are you dead against
>>> this idea now?
>>
>> No, better commit description would be useful
>
> I'll prepare something.
>
>> and perhaps a more
>> generic, "is the thing in front of a URI/URL" a known/supported entry,
>> instead of using an known invalid test would be a better mechanism.
>
> Are you sure about that? It's not that I disagree completely, but it
> sounds overly restrictive to me and makes it harder for new tags to
> evolve in case we might want them.
>
> And what tags would be on this allow-list? Anything else then "Link" and
> "Patchwork"? Those are the ones that looked common and valid to me when
> I ran
>
> git log --grep='http' v4.0.. | grep http | grep -v ' Link: ' | less
>
> and skimmed the output. Maybe "Datasheet" should be allowed, too -- not
> sure.
>
> But I found a few others that likely should be on the disallow list:
> "Closes:", "Bug:", "Gitlab issue:", "References:", "Ref:", "Bugzilla:",
> "RHBZ:", and "link", as "Link" should be used instead in all of these
> cases afaics.
>
>>>>> feat: checkpatch: Warn about Reported-by: not being followed by a
>>>>> Link:
>>>>
>>>> I think this is unnecessary.
>>>
>>> I expect this to cause more discussion, which is why this should be in a
>>> separate submission. But in the end the reasons are similar as above:
>>> (1) Linus really want to see those links to make things easier for
>>> future code archeologists. (2) My regression tracking efforts heavily
>>> rely on those Links, as they allow to automatically connect reports with
>>> patches and commits to fix the reported regression; without those the
>>> tracking is a PITA and doesn't scale.
>>>
>>> Together that IMHO is strong enough reason to warn in this case.
>>
>> Maybe, I think there might be some value in not intermixing signature
>> tags and other tags though.
>
> Hmm, sorry, not sure if I understood you here. Why do you consider
> "Reported-by:" a signature tag? Isn't it more of an "appreciation" tag,
> IOW, a kind of 'thank you' hat tip to the reporter?

Sorry, now I understand[1]: you consider "Reported-by:" a signature tag
as it's used in the area where the Signed-off-by resist.

Well, yeah, maybe it shouldn't be like that[2]. But it's quite normal
afaics to use Reported-by: (with and without Link:) in that area
currently. Changing that would break habits of many kernel developers.
And having the Reported-by: and the Link: next to each other IMHO is
important, as it makes it obvious what the Link is about; that's afaics
also why Linus wants it that way, as can be seen from the quotes I
provided earlier. See also commit 0ba09b173387 from a few days ago[3],
where he afaics added the Link tags to reports of that regression himself.

Ciao, Thorsten

[1] Kai (many thx!) explained and pointed me to the area in the
checkpatch.pl code that made it obvious:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl#n611

[2] in an ideal world things like that and tags like Tested-by: would
resist in git notes or something, as then they could be extended after a
change was committed...

[3] https://git.kernel.org/torvalds/c/0ba09b173387

2022-12-06 07:50:01

by Joe Perches

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

On Tue, 2022-12-06 at 08:17 +0100, Thorsten Leemhuis wrote:
> On 06.12.22 07:27, Thorsten Leemhuis wrote:
> > On 06.12.22 06:54, Joe Perches wrote:
[]
> > > and perhaps a more
> > > generic, "is the thing in front of a URI/URL" a known/supported entry,
> > > instead of using an known invalid test would be a better mechanism.
> >
> > Are you sure about that? It's not that I disagree completely, but it
> > sounds overly restrictive to me and makes it harder for new tags to
> > evolve in case we might want them.

It's easy to add newly supported values to a list.

> > And what tags would be on this allow-list? Anything else then "Link" and
> > "Patchwork"? Those are the ones that looked common and valid to me when
> > I ran
> >
> > git log --grep='http' v4.0.. | grep http | grep -v ' Link: ' | less
> >
> > and skimmed the output. Maybe "Datasheet" should be allowed, too -- not
> > sure.
[]
> > But I found a few others that likely should be on the disallow list:
> > "Closes:", "Bug:", "Gitlab issue:", "References:", "Ref:", "Bugzilla:",
> > "RHBZ:", and "link", as "Link" should be used instead in all of these
> > cases afaics.

Do understand please that checkpatch will never be perfect.
At best, it's just a guidance tool.

To me most of these are in the noise level, but perhaps all should just
use Link:

$ git log -100000 --format=email -P --grep='^\w+:[ \t]*http' | \
grep -Poh '^\w+:[ \t]*http' | \
sort | uniq -c | sort -rn
103889 Link: http
415 BugLink: http
372 Patchwork: http
270 Closes: http
221 Bug: http
121 References: http
101 v1: http
77 Bugzilla: http
60 URL: http
59 v2: http
37 Datasheet: http
35 v3: http
19 v4: http
12 v5: http
9 Ref: http
9 Fixes: http
9 Buglink: http
8 v6: http
8 Reference: http
7 V1: http
7 See: http
6 1: http
5 link: http
4 v7: http
4 v0: http
4 0: http
3 LINK: http
3 Link:http
3 IGT: http
2 V3: http
2 Schematics: http
2 RHBZ: http
2 RFC: http
2 Reported: http
2 MR: http
2 Bugs: http
2 BUG: http
2 2: http
1 Website: http
1 V9: http
1 v9: http
1 V8: http
1 v8: http
1 V7: http
1 V6: http
1 V5: http
1 V4: http
1 V2: http
1 v1:http
1 v10: http
1 Twitter: http
1 tree: http
1 tool: http
1 tests: http
1 tasks: http
1 SPI: http
1 Source: http
1 SoM: http
1 Reproducer: http
1 reliable: http
1 Related: http
1 Reference:http
1 Mesa: http
1 Lore: http
1 Links:http
1 Links: http
1 Link: http
1 ink: http
1 in: http
1 here: http
1 bz: http
1 Bug:http
1 AlsaInfo: http

2022-12-06 09:46:15

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

On 06.12.22 08:44, Joe Perches wrote:
> On Tue, 2022-12-06 at 08:17 +0100, Thorsten Leemhuis wrote:
>> On 06.12.22 07:27, Thorsten Leemhuis wrote:
>>> On 06.12.22 06:54, Joe Perches wrote:
> []
>>>> and perhaps a more
>>>> generic, "is the thing in front of a URI/URL" a known/supported entry,
>>>> instead of using an known invalid test would be a better mechanism.
>>>
>>> Are you sure about that? It's not that I disagree completely, but it
>>> sounds overly restrictive to me and makes it harder for new tags to
>>> evolve in case we might want them.
>
> It's easy to add newly supported values to a list.
>
>>> And what tags would be on this allow-list? Anything else then "Link" and
>>> "Patchwork"? Those are the ones that looked common and valid to me when
>>> I ran
>>>
>>> git log --grep='http' v4.0.. | grep http | grep -v ' Link: ' | less
>>>
>>> and skimmed the output. Maybe "Datasheet" should be allowed, too -- not
>>> sure.
> []
>>> But I found a few others that likely should be on the disallow list:
>>> "Closes:", "Bug:", "Gitlab issue:", "References:", "Ref:", "Bugzilla:",
>>> "RHBZ:", and "link", as "Link" should be used instead in all of these
>>> cases afaics.
>
> Do understand please that checkpatch will never be perfect.
> At best, it's just a guidance tool.

Of course -- and that's actually a reason why I prefer a disallow list
over an allow list, as that gives guidance in the way of "don't use this
tag, use Link instead" instead of enforcing "always use Link: when
linking somewhere" (now that I've written it like that it feels even
more odd, because it's obvious that it's a link, so why bother with a
tag; but whatever).

I also think the approach with a disallow list will not bother
developers much, while the other forces them a bit to much into a scheme.

> To me most of these are in the noise level, but perhaps all should just
> use Link:
>
> $ git log -100000 --format=email -P --grep='^\w+:[ \t]*http' | \
> grep -Poh '^\w+:[ \t]*http' | \
> sort | uniq -c | sort -rn
> 103889 Link: http
> 415 BugLink: http
> 372 Patchwork: http
> 270 Closes: http
> 221 Bug: http
> 121 References: http
> [...]

Ha, I considered doing something like that when I wrote my earlier mail,
but was to lazy. :-D thx!

Yeah, they are not that often, but I grew tired arguing about that,
that's why I think checkpatch is the better place and in the better
position to handle that.

Anyway, so how to move forward now? Do you insist on a allow list (IOW:
a Link: or Patchwork: before every http...)? Or is a disallow list with
the most common unwanted tags for links (that you thankfully compiled)
fine for you as well?

Ciao, Thorsten