[Linux-kernel-mentees] [PATCH] checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs

Lukas Bulwahn lukas.bulwahn at gmail.com
Thu Dec 17 12:42:03 UTC 2020


On Thu, Dec 17, 2020 at 12:12 PM Aditya <yashsri421 at gmail.com> wrote:

> On 16/12/20 9:59 pm, Aditya wrote:
> > On 16/12/20 6:27 pm, Lukas Bulwahn wrote:
> >> On Wed, Dec 16, 2020 at 1:54 PM Aditya <yashsri421 at gmail.com> wrote:
> >>>
> >>> On 16/12/20 6:05 pm, Lukas Bulwahn wrote:
> >>>> On Wed, Dec 16, 2020 at 1:32 PM Aditya Srivastava <
> yashsri421 at gmail.com> wrote:
> >>>>>
> >>>>> Currently checkpatch warns for long line in commit messages even for
> >>>>> URL lines.
> >>>>>
> >>>>> An evaluation over v4.13..v5.8 showed that out of ~11000 warnings for
> >>>>> this class, around 790 are due to the line containing link.
> >>>>>
> >>>>> E.g. running checkpatch on commit 3cde818cd02b ("ASoC: topology:
> >>>>> Consolidate how dtexts and dvalues are freed") reports this warning:
> >>>>>
> >>>>> WARNING: Possible unwrapped commit description (prefer a maximum 75
> chars per line)
> >>>>>
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
> >>>>>
> >>>>> Avoid giving users warning for character limit, instead suggest them
> to
> >>>>> prefix the URLs with "Link:"
> >>>>>
> >>>>
> >>>> Looks good to me. This might be also a good one for a fix option.
> >>>>
> >>>
> >>> I agree. Should I add the fix in this patch itself?
> >>>
> >>
> >> No, make two patches and send them as a patch series to the mailing
> list.
> >>
> > Hi Lukas
> > So, I generated a list of lines, which are URLs and reported by
> > COMMIT_LOG_LONG_LINE here:
> >
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task6/links/long_line_links.txt
> >
> > These occur in different forms:
> >
> > 1) A large part of these occur as \s*(http|https).
> >
> > 2) These also occur as [1]: https, [2]: https, etc
> >
> > 3) BugLink: https:// (e.g.: BugLink:
> > http://lkml.kernel.org/r/20180426045223.GA15307@dragonet.kaist.ac.kr)
> >
> > 4) References: https:// (e.g.: References:
> >
> http://mid.mail-archive.com/alpine.DEB.2.20.1905262211270.24390@whs-18.cs.helsinki.fi
> )
> >
> > 5) Link to v1: (E.g.: Link to v1:
> >
> https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-6-sean@poorly.run
> )
> >
> > 6) Datasheet (e.g.
> > Datasheet:
> http://www.winbond.com/resource-files/w25n01gv%20revl%20050918%20unsecured.pdf
> )
> >
> > 7) URL enclosed within brackets (E.g.:
> > (
> https://syzkaller.appspot.com/bug?id=ad7e0351fbc90535558514a71cd3edc11681997a)
> .)
> >
> > 8) Link within sentences (E.g.: suite can be found at
> > https://github.com/Cyan4973/xxHash/blob/cf46e0c/xxhsum.c#L664)
> >
> > etc.
> >
> > Among these I guess we can fix occurrences like 1st by prefixing with
> > "Link:". But I am not sure about the rest.
> >
> > What do you think?
> >
>
> I think we can only fix the lines with ^\s*(http|https|ftp) and should
> just give warning for the others. The user can then decide how they
> want to fix.
>
> Or should we give warning also for ^\s*(http|https|ftp) only?
>
> I guess we can send this patch to Joe and see what he thinks. This
> might give us a better idea for the fix.
>
> What do you think?
>
>
Agree. If we summarize the use of hyperlinks in commit messages properly,
we might also ask on the ksummit-discuss list for some feedback on
"standardizing the use of hyperlinks in commit messages beyond Link-tags".

All those examples show that there are various different preferences but
really no common agreed way that everyone uses.

Let us start the discussion with Joe and then see.

Lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxfoundation.org/pipermail/linux-kernel-mentees/attachments/20201217/d1818169/attachment.html>


More information about the Linux-kernel-mentees mailing list