[Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags

[Lukas Bulwahn]

On Thu, Nov 12, 2020 at 10:39 AM Aditya <yashsri421 at gmail.com> wrote:
>
> On 12/11/20 1:24 am, Lukas Bulwahn wrote:
> > On Wed, Nov 11, 2020 at 8:35 PM Aditya <yashsri421 at gmail.com> wrote:
> >>
> >> On 12/11/20 12:56 am, Lukas Bulwahn wrote:
> >>> On Wed, Nov 11, 2020 at 8:20 PM Aditya Srivastava <yashsri421 at gmail.com> wrote:
> >>>>
> >>>> Currently checkpatch warns us for long lines in commits even for
> >>>> signature tag lines.
> >>>>
> >>>> E.g., running checkpatch on commit 4bbdfe25600c ("IB/opa_vnic: Properly
> >>>> clear Mac Table Digest") reports this warning:
> >>>>
> >>>> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> >>>> Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
> >>>>
> >>>> This is an example of false positive. Provide a fix by excluding
> >>>> signature tag lines from this class of warning.
> >>>>
> >>>> Signed-off-by: Aditya Srivastava <yashsri421 at gmail.com>
> >>>> ---
> >>>>  scripts/checkpatch.pl | 2 ++
> >>>>  1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >>>> index ac7e5ac80b58..1d257b1f4a4f 100755
> >>>> --- a/scripts/checkpatch.pl
> >>>> +++ b/scripts/checkpatch.pl
> >>>> @@ -2963,6 +2963,8 @@ sub process {
> >>>>                                         # filename then :
> >>>>                       $line =~ /^\s*(?:Fixes:|Link:)/i ||
> >>>>                                         # A Fixes: or Link: line
> >>>> +                     $line =~ /^\s*(?:$signature_tags)/ ||
> >>>> +                                       # signature tag line
> >>>
> >>> Can you simply add this check to the expression above?
> >>>
> >> Yes, we can do it. But then the match will become case insensitive
> >> (because of presence of '/i'). Should I do it irrespective?
> >>
> >
> > I see. Then keep it as is and start the evaluation.
> >
>
> I evaluated this patch on v5.6..v5.8 and generated the list of dropped
> warnings for commit_log_long_line here:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/signature_tags/dropped/summary.txt
>
> The difference in frequency is 17 (ie from 1896 to 1879)
>

Thanks, let us add this summary to the commit message;

Also I think we can drop the specific example and simply summarize why
the 17 cases are beyond 75 characters.

- Long names and long email addresses
- some comments on scoped review and acknowledgement, i.e., for a
dedicated pointer on what was reported by the identity in Reported-by:
- some additional comments on CC: stable at vger.kernel.org tags

You can also shorten this:

 This is an example of false positive. Provide a fix by excluding
signature tag lines from this class of warning.

to:

Exclude signature tag lines from this class of warning.


Send a reworked v2 for a quick review; then, you can probably send it
to Joe and lkml; as follow-up reply to your patch, you can direct add
the link to

 https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/signature_tags/dropped/summary.txt

Joe Perches can then quickly cross-check that evaluation as well.

Lukas