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

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

[Aditya Srivastava]

Currently checkpatch warns us for long lines in commits even for
signature tag lines.

Generally these lines exceed 75 characters limit because of:
1) long names and long email address
2) some comments on scoped review and acknowledgement, i.e., for a
dedicated pointer on what was reported by the identity in 'Reported-by'
3) some additional comments on CC: stable@vger.org tags

Exclude signature tag lines from this class of warning.

A quick evaluation on v5.6..v5.8 showed that this fix reduces this
warning by a frequency of 17.

A quick manual check found all the dropped warnings related to signature
tags.

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
 scripts/checkpatch.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ac7e5ac80b58..f2369ac29d50 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2961,8 +2961,8 @@ sub process {
 					# file delta changes
 		      $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ ||
 					# filename then :
-		      $line =~ /^\s*(?:Fixes:|Link:)/i ||
-					# A Fixes: or Link: line
+		      $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
+					# A Fixes: or Link: line or 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

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

[Aditya]

On 13/11/20 1:28 am, Aditya Srivastava wrote:
> Currently checkpatch warns us for long lines in commits even for
> signature tag lines.
> 
> Generally these lines exceed 75 characters limit because of:
> 1) long names and long email address
> 2) some comments on scoped review and acknowledgement, i.e., for a
> dedicated pointer on what was reported by the identity in 'Reported-by'
> 3) some additional comments on CC: stable@vger.org tags
> 
> Exclude signature tag lines from this class of warning.
> 
> A quick evaluation on v5.6..v5.8 showed that this fix reduces this
> warning by a frequency of 17.
> 
> A quick manual check found all the dropped warnings related to signature
> tags.
> 
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
>  scripts/checkpatch.pl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index ac7e5ac80b58..f2369ac29d50 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2961,8 +2961,8 @@ sub process {
>  					# file delta changes
>  		      $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ ||
>  					# filename then :
> -		      $line =~ /^\s*(?:Fixes:|Link:)/i ||
> -					# A Fixes: or Link: line
> +		      $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
> +					# A Fixes: or Link: line or 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);
> 

I think this patch probably got missed. Replying for your attention :)
Pardon me if it was intentional.

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

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

[Lukas Bulwahn]

On Thu, Nov 12, 2020 at 8:58 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently checkpatch warns us for long lines in commits even for
> signature tag lines.
>
> Generally these lines exceed 75 characters limit because of:

... exceed the 75-character limit because of:

[yes, English is strange ;)]

> 1) long names and long email address
> 2) some comments on scoped review and acknowledgement, i.e., for a
> dedicated pointer on what was reported by the identity in 'Reported-by'
> 3) some additional comments on CC: stable@vger.org tags
>
> Exclude signature tag lines from this class of warning.
>
> A quick evaluation on v5.6..v5.8 showed that this fix reduces this
> warning by a frequency of 17.
>

frequency is the wrong word here. It is simply "count" here.

Also how many warnings were there before and how many after?

The count of 17 does not tell you much on the relative impact of the change.


> A quick manual check found all the dropped warnings related to signature
> tags.
>

What did the quick manual check found?
Do you mean the quick manual check found all ... to be false positive
that should not have been warned about?

Other than that, it looks good.

Lukas

> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
>  scripts/checkpatch.pl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index ac7e5ac80b58..f2369ac29d50 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2961,8 +2961,8 @@ sub process {
>                                         # file delta changes
>                       $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ ||
>                                         # filename then :
> -                     $line =~ /^\s*(?:Fixes:|Link:)/i ||
> -                                       # A Fixes: or Link: line
> +                     $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
> +                                       # A Fixes: or Link: line or 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

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

[Aditya]

On 15/11/20 8:55 pm, Lukas Bulwahn wrote:
> On Thu, Nov 12, 2020 at 8:58 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>
>> Currently checkpatch warns us for long lines in commits even for
>> signature tag lines.
>>
>> Generally these lines exceed 75 characters limit because of:
> 
> ... exceed the 75-character limit because of:
> 
> [yes, English is strange ;)]
> 
>> 1) long names and long email address
>> 2) some comments on scoped review and acknowledgement, i.e., for a
>> dedicated pointer on what was reported by the identity in 'Reported-by'
>> 3) some additional comments on CC: stable@vger.org tags
>>
>> Exclude signature tag lines from this class of warning.
>>
>> A quick evaluation on v5.6..v5.8 showed that this fix reduces this
>> warning by a frequency of 17.
>>
> 
> frequency is the wrong word here. It is simply "count" here.
> 
Okay.

> Also how many warnings were there before and how many after?
> 
> The count of 17 does not tell you much on the relative impact of the change.
> 
> 

So, before this patch, COMMIT_LOG_LONG_LINE had frequency of 1896 and
after the patch, it becomes 1879 (over v5.6..v5.8). Should I add this
information in commit message as well

>> A quick manual check found all the dropped warnings related to signature
>> tags.
>>
> 
> What did the quick manual check found?
> Do you mean the quick manual check found all ... to be false positive
> that should not have been warned about?
> 

Yes, all the dropped warnings are prefixed by signature tags.
Generated list:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/signature_tags/dropped/summary.txt

> Other than that, it looks good.
> 
> Lukas
> 
>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>> ---
>>  scripts/checkpatch.pl | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index ac7e5ac80b58..f2369ac29d50 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -2961,8 +2961,8 @@ sub process {
>>                                         # file delta changes
>>                       $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ ||
>>                                         # filename then :
>> -                     $line =~ /^\s*(?:Fixes:|Link:)/i ||
>> -                                       # A Fixes: or Link: line
>> +                     $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
>> +                                       # A Fixes: or Link: line or 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

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

[Lukas Bulwahn]

On Sun, Nov 15, 2020 at 6:00 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 15/11/20 8:55 pm, Lukas Bulwahn wrote:
> > On Thu, Nov 12, 2020 at 8:58 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
> >>
> >> Currently checkpatch warns us for long lines in commits even for
> >> signature tag lines.
> >>
> >> Generally these lines exceed 75 characters limit because of:
> >
> > ... exceed the 75-character limit because of:
> >
> > [yes, English is strange ;)]
> >
> >> 1) long names and long email address
> >> 2) some comments on scoped review and acknowledgement, i.e., for a
> >> dedicated pointer on what was reported by the identity in 'Reported-by'
> >> 3) some additional comments on CC: stable@vger.org tags
> >>
> >> Exclude signature tag lines from this class of warning.
> >>
> >> A quick evaluation on v5.6..v5.8 showed that this fix reduces this
> >> warning by a frequency of 17.
> >>
> >
> > frequency is the wrong word here. It is simply "count" here.
> >
> Okay.
>
> > Also how many warnings were there before and how many after?
> >
> > The count of 17 does not tell you much on the relative impact of the change.
> >
> >
>
> So, before this patch, COMMIT_LOG_LONG_LINE had frequency of 1896 and
> after the patch, it becomes 1879 (over v5.6..v5.8). Should I add this
> information in commit message as well
>

And I repeat: "frequency" is the wrong word; this is NOT a frequency.

Just write:

There were 1896 COMMIT_LOG_LONG_LINE warnings in v5.6..v5.8 before
this patch application and 1879 afterwards.

You are using the word frequency wrongly. It is probably best to just
remove that word from your vocabulary for now.

Send a new patch with a corrected description and I will review.


> >> A quick manual check found all the dropped warnings related to signature
> >> tags.
> >>
> >
> > What did the quick manual check found?
> > Do you mean the quick manual check found all ... to be false positive
> > that should not have been warned about?
> >
>
> Yes, all the dropped warnings are prefixed by signature tags.
> Generated list:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/signature_tags/dropped/summary.txt
>
> > Other than that, it looks good.
> >
> > Lukas
> >
> >> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> >> ---
> >>  scripts/checkpatch.pl | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >> index ac7e5ac80b58..f2369ac29d50 100755
> >> --- a/scripts/checkpatch.pl
> >> +++ b/scripts/checkpatch.pl
> >> @@ -2961,8 +2961,8 @@ sub process {
> >>                                         # file delta changes
> >>                       $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ ||
> >>                                         # filename then :
> >> -                     $line =~ /^\s*(?:Fixes:|Link:)/i ||
> >> -                                       # A Fixes: or Link: line
> >> +                     $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
> >> +                                       # A Fixes: or Link: line or 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