2023-05-15 19:46:49

by Francesco Dolcini

[permalink] [raw]
Subject: Reported-by checkpatch warning from v6.4-rc1

Hello,
starting from commit d6ccdd678e45 ("checkpatch: check for misuse of the link tags")
any Reported-by: tag not followed by a Closes trigger a warning, even if
we have a Link: tag afterward.

Example:

WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#8:
Reported-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
total: 0 errors, 1 warnings, 8 lines checked

From what I can understand it was not in the intention of that patch,
and Link: is still fine to be used.

Comments?

Francesco




2023-05-16 10:08:32

by Matthieu Baerts

[permalink] [raw]
Subject: Re: Reported-by checkpatch warning from v6.4-rc1

Hi Francesco,

On 15/05/2023 21:17, Francesco Dolcini wrote:
> Hello,
> starting from commit d6ccdd678e45 ("checkpatch: check for misuse of the link tags")
> any Reported-by: tag not followed by a Closes trigger a warning, even if
> we have a Link: tag afterward.

The warning you are mentioning below is likely due to commit
44c31888098a ("checkpatch: allow Closes tags with links") instead I think.

This commit is linked to the modification of the documentation allowing
the Closes tag, see commit 0d828200ad56 ("docs: process: allow Closes
tags with links") for more details.

Initially, the intension was to allow using the Closes tags with links
to public bugs trackers because in 6.3, checkpatch was displaying
warnings when it was used while it was fine before. But it turned out
that this Closes tag can be useful for bots tracking bugs reported on
public mailing lists. As explained in [1] and [2], various bug trackers
can use this tag to trigger some actions. It is then helpful to use the
Closes tag each time a commit fixes a bug reported somewhere.

[1]
https://lore.kernel.org/all/20230314-doc-checkpatch-closes-tag-v4-0-d26d1fa66f9f@tessares.net/T/
[2] https://docs.kernel.org/process/submitting-patches.html

> Example:
>
> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> #8:
> Reported-by: kernel test robot <[email protected]>
> Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> total: 0 errors, 1 warnings, 8 lines checked
The warning is saying to replace the "Link:" tag by a "Closes:" one
here. Is it an issue to do that here?

I guess it means that the kernel test robot should then suggest to use
the "Closes:" tag. I can report an issue on their bugs tracker.

> From what I can understand it was not in the intention of that patch,
> and Link: is still fine to be used.

Note that Checkpatch cannot cover all cases: it gives you
recommendations but they can be ignored (and documented in the commit
message) if they don't make sense in your case. Here, it looks like you
should use the Closes tag but it might be fine to use the Link tag if a
commit doesn't fully fix the reported issue for example.

Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net

2023-05-17 01:19:58

by Li, Philip

[permalink] [raw]
Subject: Re: Reported-by checkpatch warning from v6.4-rc1

On Tue, May 16, 2023 at 11:47:45AM +0200, Matthieu Baerts wrote:
> Hi Francesco,
>
> On 15/05/2023 21:17, Francesco Dolcini wrote:
> > Hello,
> > starting from commit d6ccdd678e45 ("checkpatch: check for misuse of the link tags")
> > any Reported-by: tag not followed by a Closes trigger a warning, even if
> > we have a Link: tag afterward.
>
> The warning you are mentioning below is likely due to commit
> 44c31888098a ("checkpatch: allow Closes tags with links") instead I think.
>
> This commit is linked to the modification of the documentation allowing
> the Closes tag, see commit 0d828200ad56 ("docs: process: allow Closes
> tags with links") for more details.
>
> Initially, the intension was to allow using the Closes tags with links
> to public bugs trackers because in 6.3, checkpatch was displaying
> warnings when it was used while it was fine before. But it turned out
> that this Closes tag can be useful for bots tracking bugs reported on
> public mailing lists. As explained in [1] and [2], various bug trackers
> can use this tag to trigger some actions. It is then helpful to use the
> Closes tag each time a commit fixes a bug reported somewhere.
>
> [1]
> https://lore.kernel.org/all/20230314-doc-checkpatch-closes-tag-v4-0-d26d1fa66f9f@tessares.net/T/
> [2] https://docs.kernel.org/process/submitting-patches.html
>
> > Example:
> >
> > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > #8:
> > Reported-by: kernel test robot <[email protected]>
> > Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > total: 0 errors, 1 warnings, 8 lines checked
> The warning is saying to replace the "Link:" tag by a "Closes:" one
> here. Is it an issue to do that here?
>
> I guess it means that the kernel test robot should then suggest to use
> the "Closes:" tag. I can report an issue on their bugs tracker.

Hi Mat, thanks a lot for the suggestion, use Closes tag is clearer here,
we will update the report info accordingly.

>
> > From what I can understand it was not in the intention of that patch,
> > and Link: is still fine to be used.
>
> Note that Checkpatch cannot cover all cases: it gives you
> recommendations but they can be ignored (and documented in the commit
> message) if they don't make sense in your case. Here, it looks like you
> should use the Closes tag but it might be fine to use the Link tag if a
> commit doesn't fully fix the reported issue for example.
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> http://www.tessares.net