Thread (10 messages) 10 messages, 3 authors, 2020-11-12

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

From: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Date: 2020-11-12 18:00:26

On Thu, Nov 12, 2020 at 6:35 PM Aditya [off-list ref] wrote:
On 12/11/20 10:10 pm, Dwaipayan Ray wrote:
quoted
On 12/11/20 1:05 am, Aditya wrote:
quoted
On 12/11/20 12:56 am, Lukas Bulwahn wrote:
quoted
On Wed, Nov 11, 2020 at 8:20 PM Aditya Srivastava
[off-list ref] wrote:
quoted
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
[off-list ref]

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 <redacted>
---
  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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help