All of lore.kernel.org
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
@ 2020-11-11 19:19 Aditya Srivastava
  2020-11-11 19:23 ` Aditya
  2020-11-11 19:26 ` Lukas Bulwahn
  0 siblings, 2 replies; 10+ messages in thread
From: Aditya Srivastava @ 2020-11-11 19:19 UTC (permalink / raw)
  To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421

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@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@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
 		      $commit_log_possible_stack_dump)) {
 			WARN("COMMIT_LOG_LONG_LINE",
 			     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
-- 
2.17.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-11 19:19 [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags Aditya Srivastava
@ 2020-11-11 19:23 ` Aditya
  2020-11-11 19:26 ` Lukas Bulwahn
  1 sibling, 0 replies; 10+ messages in thread
From: Aditya @ 2020-11-11 19:23 UTC (permalink / raw)
  To: lukas.bulwahn; +Cc: linux-kernel-mentees

On 12/11/20 12:49 am, Aditya Srivastava 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@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@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
>  		      $commit_log_possible_stack_dump)) {
>  			WARN("COMMIT_LOG_LONG_LINE",
>  			     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> 

These are the changes I have made for avoiding signature tags from
commit_log_log_line warning. Wanted to get a quick feedback on it
before proceeding for evaluations.

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-11 19:19 [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags Aditya Srivastava
  2020-11-11 19:23 ` Aditya
@ 2020-11-11 19:26 ` Lukas Bulwahn
  2020-11-11 19:35   ` Aditya
  1 sibling, 1 reply; 10+ messages in thread
From: Lukas Bulwahn @ 2020-11-11 19:26 UTC (permalink / raw)
  To: Aditya Srivastava; +Cc: linux-kernel-mentees

On Wed, Nov 11, 2020 at 8:20 PM Aditya Srivastava <yashsri421@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@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@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?

>                       $commit_log_possible_stack_dump)) {
>                         WARN("COMMIT_LOG_LONG_LINE",
>                              "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-11 19:26 ` Lukas Bulwahn
@ 2020-11-11 19:35   ` Aditya
  2020-11-11 19:54     ` Lukas Bulwahn
  2020-11-12 16:40     ` Dwaipayan Ray
  0 siblings, 2 replies; 10+ messages in thread
From: Aditya @ 2020-11-11 19:35 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

On 12/11/20 12:56 am, Lukas Bulwahn wrote:
> On Wed, Nov 11, 2020 at 8:20 PM Aditya Srivastava <yashsri421@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@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@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

>>                       $commit_log_possible_stack_dump)) {
>>                         WARN("COMMIT_LOG_LONG_LINE",
>>                              "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
>> --
>> 2.17.1
>>

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-11 19:35   ` Aditya
@ 2020-11-11 19:54     ` Lukas Bulwahn
  2020-11-12  9:39       ` Aditya
  2020-11-12 16:40     ` Dwaipayan Ray
  1 sibling, 1 reply; 10+ messages in thread
From: Lukas Bulwahn @ 2020-11-11 19:54 UTC (permalink / raw)
  To: Aditya; +Cc: linux-kernel-mentees

On Wed, Nov 11, 2020 at 8:35 PM Aditya <yashsri421@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@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@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@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.

Lukas

> Thanks
> Aditya
>
> >>                       $commit_log_possible_stack_dump)) {
> >>                         WARN("COMMIT_LOG_LONG_LINE",
> >>                              "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> >> --
> >> 2.17.1
> >>
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-11 19:54     ` Lukas Bulwahn
@ 2020-11-12  9:39       ` Aditya
  2020-11-12 10:19         ` Lukas Bulwahn
  0 siblings, 1 reply; 10+ messages in thread
From: Aditya @ 2020-11-12  9:39 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

On 12/11/20 1:24 am, Lukas Bulwahn wrote:
> On Wed, Nov 11, 2020 at 8:35 PM Aditya <yashsri421@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@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@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@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
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-12  9:39       ` Aditya
@ 2020-11-12 10:19         ` Lukas Bulwahn
  0 siblings, 0 replies; 10+ messages in thread
From: Lukas Bulwahn @ 2020-11-12 10:19 UTC (permalink / raw)
  To: Aditya; +Cc: linux-kernel-mentees

On Thu, Nov 12, 2020 at 10:39 AM Aditya <yashsri421@gmail.com> wrote:
>
> On 12/11/20 1:24 am, Lukas Bulwahn wrote:
> > On Wed, Nov 11, 2020 at 8:35 PM Aditya <yashsri421@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@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@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@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@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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-11 19:35   ` Aditya
  2020-11-11 19:54     ` Lukas Bulwahn
@ 2020-11-12 16:40     ` Dwaipayan Ray
  2020-11-12 17:35       ` Aditya
  1 sibling, 1 reply; 10+ messages in thread
From: Dwaipayan Ray @ 2020-11-12 16:40 UTC (permalink / raw)
  To: Aditya, Lukas Bulwahn; +Cc: Dwaipayan Ray, linux-kernel-mentees


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@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@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@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.

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-12 16:40     ` Dwaipayan Ray
@ 2020-11-12 17:35       ` Aditya
  2020-11-12 18:00         ` Lukas Bulwahn
  0 siblings, 1 reply; 10+ messages in thread
From: Aditya @ 2020-11-12 17:35 UTC (permalink / raw)
  To: Dwaipayan Ray, Lukas Bulwahn; +Cc: linux-kernel-mentees

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@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@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@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?

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags
  2020-11-12 17:35       ` Aditya
@ 2020-11-12 18:00         ` Lukas Bulwahn
  0 siblings, 0 replies; 10+ messages in thread
From: Lukas Bulwahn @ 2020-11-12 18:00 UTC (permalink / raw)
  To: Aditya; +Cc: Dwaipayan Ray, linux-kernel-mentees

On Thu, Nov 12, 2020 at 6:35 PM Aditya <yashsri421@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@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@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@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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-12 18:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 19:19 [Linux-kernel-mentees] [PATCH] checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags Aditya Srivastava
2020-11-11 19:23 ` Aditya
2020-11-11 19:26 ` Lukas Bulwahn
2020-11-11 19:35   ` Aditya
2020-11-11 19:54     ` Lukas Bulwahn
2020-11-12  9:39       ` Aditya
2020-11-12 10:19         ` Lukas Bulwahn
2020-11-12 16:40     ` Dwaipayan Ray
2020-11-12 17:35       ` Aditya
2020-11-12 18:00         ` Lukas Bulwahn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.