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

[Lukas Bulwahn]

On Thu, Nov 12, 2020 at 6:35 PM Aditya <yashsri421 at gmail.com> wrote:
>
> On 12/11/20 10:10 pm, Dwaipayan Ray wrote:
> >
> > On 12/11/20 1:05 am, Aditya 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?
> >>
> >> Thanks
> >> Aditya
> >>
> > I think the check might be better as case insensitive because:
> >
> > $ git log --format=email -100000 | grep "^CC:" | wc -l
> > 773
> >
> > $ git log --format=email -100000 | grep "^Cc:" | wc -l
> > 59475
> >
> > $ git log --format=email -100000 | grep "^cc:" | wc -l
> > 95
> >
> > All three cases were used for Cc: ...
> >
> > Thanks,
> > Dwaipayan.
> >
>
> Thanks Dwaipayan. You seem to be correct. I just observed, we are
> already using '/xi' flags with $signature_tags values itself. So, case
> insensitive part seems already covered.
>
> I think I can modify the changes to include this check in the previous
> if-block itself as Sir had suggested initially. This shouldn't change
> our evaluation as well.
>
> Sir, should I make the changes as you suggested initially or should we
> proceed with it?
>

Make it case-insensitive and include it in the condition above.

Please write Lukas not Sir; that simply confuses me.

Lukas