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

[Thorsten Leemhuis]

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 <kai@xxxxxxxxxxxxxxxxxxxxxx>
>>>> To: linux-kernel@xxxxxxxxxxxxxxx
>>>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Thorsten Leemhuis <linux@xxxxxxxxxxxxx>
>>>> 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 <linux@xxxxxxxxxxxxx>
>>>> Signed-off-by: Kai Wasserbäch <kai@xxxxxxxxxxxxxxxxxxxxxx>
>>>>
>>>> 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