[Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature

[Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.

A large number of these warnings occur because of typo mistakes in
signoffs.
An evaluation over v4.13..v5.8 revealed that out of 539 warnings due to
Non-standard signatures, 85 are due to typo mistakes.

Eg. running checkpatch on commit da785a87787c ("ARM: bcm2835: Fix
integer overflow in rpi_firmware_print_firmware_revision()") reports
this warning:

WARNING:Non-standard signature: Revieved-by:
Revieved-by: Petr Mladek <pmladek@suse.com>

Here the signoff 'Reviewed-by' is misspelt.

Provide a fix by calculating levenshtein distance for the signoff over
all the standard signatures and suggest a fix if the distance for any
signature is less than or equal to 2.

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
applied on my last patch and next-20201120

 scripts/checkpatch.pl | 73 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b018deecec1a..2198360eebbd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+sub get_min {
+	my (@arr) = @_;
+	my $len = scalar @arr;
+	if((scalar @arr) < 1) {
+		# if underflow, return
+		return;
+	}
+	my $min = $arr[0];
+	for my $i (0 .. ($len-1)) {
+		if ($arr[$i] < $min) {
+			$min = $arr[$i];
+		}
+	}
+	return $min;
+}
+
+sub get_edit_distance {
+	my ($str1, $str2) = @_;
+	my $len1 = length($str1);
+	my $len2 = length($str2);
+	# two dimensional array storing minimum edit distance
+	my @distance;
+	for my $i (0 .. $len1) {
+		for my $j (0 .. $len2) {
+			if ($i == 0) {
+				$distance[$i][$j] = $j;
+			}
+			elsif ($j == 0) {
+				$distance[$i][$j] = $i;
+			}
+			elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+				$distance[$i][$j] = $distance[$i - 1][$j - 1];
+			}
+			else {
+				my $dist1 = $distance[$i][$j - 1]; #insert distance
+				my $dist2 = $distance[$i - 1][$j]; # remove
+				my $dist3 = $distance[$i - 1][$j - 1]; #replace
+				$distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+			}
+		}
+	}
+	return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+	my ($sign_off) = @_;
+	$sign_off = lc($sign_off);
+	$sign_off =~ s/\-//g; # to match with formed hash
+	my @standard_signature_tags = (
+		'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+		'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
+	);
+	# setting default values
+	my $standard_signature = 'signed-off-by';
+	my $min_edit_distance = 20;
+	my $edit_distance;
+	foreach (@standard_signature_tags) {
+		my $signature = $_;
+		$_ =~ s/\-//g;
+		$edit_distance = get_edit_distance($sign_off, $_);
+		if ($edit_distance < $min_edit_distance) {
+			$min_edit_distance = $edit_distance;
+			$standard_signature = $signature;
+		}
+	}
+        if($min_edit_distance<=2) {
+		return ucfirst($standard_signature);
+        }
+	return "";
+}
+
 our %standard_signature_fix = (
 	"Requested-by:" => {
 		suggestion => "Suggested-by:",
@@ -2848,7 +2919,7 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				my $suggested_signature = "";
+				my $suggested_signature = get_standard_signature($sign_off);
 				my $rationale = "";
 				if (exists($standard_signature_fix{$sign_off})) {
 					$suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
-- 
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] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Tue, Nov 24, 2020 at 12:17 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signoffs.
> An evaluation over v4.13..v5.8 revealed that out of 539 warnings due to
> Non-standard signatures, 85 are due to typo mistakes.
>
> Eg. running checkpatch on commit da785a87787c ("ARM: bcm2835: Fix
> integer overflow in rpi_firmware_print_firmware_revision()") reports
> this warning:
>
> WARNING:Non-standard signature: Revieved-by:
> Revieved-by: Petr Mladek <pmladek@suse.com>
>
> Here the signoff 'Reviewed-by' is misspelt.
>

Again, it is better to provide a summary of the overall evaluation
instead of a single example,

> Provide a fix by calculating levenshtein distance for the signoff over

it is a signature tag, not the signoff.

> all the standard signatures and suggest a fix if the distance for any
> signature is less than or equal to 2.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> applied on my last patch and next-20201120
>

Joe, might ack this one and reject the other; so you might need to rebase later.

For the discussion with Joe now that is fine, though.

Other than that, it looks good.

>  scripts/checkpatch.pl | 73 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b018deecec1a..2198360eebbd 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +sub get_min {
> +       my (@arr) = @_;
> +       my $len = scalar @arr;
> +       if((scalar @arr) < 1) {
> +               # if underflow, return
> +               return;
> +       }
> +       my $min = $arr[0];
> +       for my $i (0 .. ($len-1)) {
> +               if ($arr[$i] < $min) {
> +                       $min = $arr[$i];
> +               }
> +       }
> +       return $min;
> +}
> +
> +sub get_edit_distance {
> +       my ($str1, $str2) = @_;
> +       my $len1 = length($str1);
> +       my $len2 = length($str2);
> +       # two dimensional array storing minimum edit distance
> +       my @distance;
> +       for my $i (0 .. $len1) {
> +               for my $j (0 .. $len2) {
> +                       if ($i == 0) {
> +                               $distance[$i][$j] = $j;
> +                       }
> +                       elsif ($j == 0) {
> +                               $distance[$i][$j] = $i;
> +                       }
> +                       elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
> +                               $distance[$i][$j] = $distance[$i - 1][$j - 1];
> +                       }
> +                       else {
> +                               my $dist1 = $distance[$i][$j - 1]; #insert distance
> +                               my $dist2 = $distance[$i - 1][$j]; # remove
> +                               my $dist3 = $distance[$i - 1][$j - 1]; #replace
> +                               $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
> +                       }
> +               }
> +       }
> +       return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> +       my ($sign_off) = @_;
> +       $sign_off = lc($sign_off);
> +       $sign_off =~ s/\-//g; # to match with formed hash
> +       my @standard_signature_tags = (
> +               'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
> +               'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
> +       );
> +       # setting default values
> +       my $standard_signature = 'signed-off-by';
> +       my $min_edit_distance = 20;
> +       my $edit_distance;
> +       foreach (@standard_signature_tags) {
> +               my $signature = $_;
> +               $_ =~ s/\-//g;
> +               $edit_distance = get_edit_distance($sign_off, $_);
> +               if ($edit_distance < $min_edit_distance) {
> +                       $min_edit_distance = $edit_distance;
> +                       $standard_signature = $signature;
> +               }
> +       }
> +        if($min_edit_distance<=2) {
> +               return ucfirst($standard_signature);
> +        }
> +       return "";
> +}
> +
>  our %standard_signature_fix = (
>         "Requested-by:" => {
>                 suggestion => "Suggested-by:",
> @@ -2848,7 +2919,7 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               my $suggested_signature = "";
> +                               my $suggested_signature = get_standard_signature($sign_off);
>                                 my $rationale = "";
>                                 if (exists($standard_signature_fix{$sign_off})) {
>                                         $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v2] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.

A large number of these warnings occur because of typo mistakes in
signoffs. An evaluation over v4.13..v5.8 revealed that out of 539
warnings due to Non-standard signatures, 85 are due to typo mistakes.

Provide a fix by calculating levenshtein distance for the signature tag
over all the standard signatures and suggest a fix with signature, whose
edit distance is less than or equal to 2.

According to evaluation over v4.13..v5.8, following are the incorrect
signature tags with typos and their corresponding suggested fixes
(according to edit distace approach):

1)Reviwed-by (count: 19) => Reviewed-by

2)Reviewd-by (count: 9) => Reviewed-by

3)Singed-off-by (count: 8) => Signed-off-by

4)Signed-of-by (count: 6) => Signed-off-by

5)Rewieved-by (count: 3) => Reviewed-by

6)Signed-off--by (count: 3) => Signed-off-by

7)Revieved-by (count: 3) => Reviewed-by

8)Reivewed-by (count: 2) => Reviewed-by

9)Signef-off-by (count: 2) => Signed-off-by

10)Test-by (count: 2) => Tested-by

11)Acked_by (count: 2) => Acked-by

12)Signed-off-by-by (count: 2) => Signed-off-by

13)Reported-by-by (count: 1) => Reported-by

14)Reporetd-by (count: 1) => Reported-by

15)Reviewed--by (count: 1) => Reviewed-by

16)Sugested-by (count: 1) => Suggested-by

17)Suggested--by (count: 1) => Suggested-by

18)Repoted-by (count: 1) => Reported-by

19)Rported-by (count: 1) => Reported-by

20)eigned-off-by (count: 1) => Signed-off-by

21)Reveiwed-by (count: 1) => Reviewed-by

22)igned-off-by (count: 1) => Signed-off-by

23)Tested-by-by (count: 1) => Tested-by

24)Sugessted-by (count: 1) => Suggested-by

25)Rewiewed-by (count: 1) => Reviewed-by

26)Teste-by (count: 1) => Tested-by

27)Signee-off-by (count: 1) => Signed-off-by

28)-By (count: 1) => To

29)Signen-off-by (count: 1) => Signed-off-by

30)eported-by (count: 1) => Reported-by

31)Reviewedy-by (count: 1) => Signed-off-by

32)Siganed-off-by (count: 1) => Signed-off-by

33)Ackedy-by (count: 1) => Acked-by

34)Review-by (count: 1) => Reviewed-by

35)Tweeted-by (count: 1) => Tested-by

36)Ack-by (count: 1) => Acked-by

37)Reorted-by (count: 1) => Reported-by

38)Suggsted-by (count: 1) => Suggested-by

39)Reviwed-By (count: 1) => Reviewed-by

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
patch applies on next-20201120

 scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..775a49a06179 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+sub get_min {
+	my (@arr) = @_;
+	my $len = scalar @arr;
+	if((scalar @arr) < 1) {
+		# if underflow, return
+		return;
+	}
+	my $min = $arr[0];
+	for my $i (0 .. ($len-1)) {
+		if ($arr[$i] < $min) {
+			$min = $arr[$i];
+		}
+	}
+	return $min;
+}
+
+sub get_edit_distance {
+	my ($str1, $str2) = @_;
+	my $len1 = length($str1);
+	my $len2 = length($str2);
+	# two dimensional array storing minimum edit distance
+	my @distance;
+	for my $i (0 .. $len1) {
+		for my $j (0 .. $len2) {
+			if ($i == 0) {
+				$distance[$i][$j] = $j;
+			}
+			elsif ($j == 0) {
+				$distance[$i][$j] = $i;
+			}
+			elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+				$distance[$i][$j] = $distance[$i - 1][$j - 1];
+			}
+			else {
+				my $dist1 = $distance[$i][$j - 1]; #insert distance
+				my $dist2 = $distance[$i - 1][$j]; # remove
+				my $dist3 = $distance[$i - 1][$j - 1]; #replace
+				$distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+			}
+		}
+	}
+	return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+	my ($sign_off) = @_;
+	$sign_off = lc($sign_off);
+	$sign_off =~ s/\-//g; # to match with formed hash
+	my @standard_signature_tags = (
+		'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+		'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
+	);
+	# setting default values
+	my $standard_signature = 'signed-off-by';
+	my $min_edit_distance = 20;
+	my $edit_distance;
+	foreach (@standard_signature_tags) {
+		my $signature = $_;
+		$_ =~ s/\-//g;
+		$edit_distance = get_edit_distance($sign_off, $_);
+		if ($edit_distance < $min_edit_distance) {
+			$min_edit_distance = $edit_distance;
+			$standard_signature = $signature;
+		}
+	}
+        if($min_edit_distance<=2) {
+		return ucfirst($standard_signature);
+        }
+	return "";
+}
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2844,18 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = get_standard_signature($sign_off);
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					"Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						 "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
+					    $fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
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: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Tue, Nov 24, 2020 at 4:32 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signoffs. An evaluation over v4.13..v5.8 revealed that out of 539
> warnings due to Non-standard signatures, 85 are due to typo mistakes.
>
> Provide a fix by calculating levenshtein distance for the signature tag
> over all the standard signatures and suggest a fix with signature, whose
> edit distance is less than or equal to 2.
>
> According to evaluation over v4.13..v5.8, following are the incorrect
> signature tags with typos and their corresponding suggested fixes
> (according to edit distace approach):
>
> 1)Reviwed-by (count: 19) => Reviewed-by
>
> 2)Reviewd-by (count: 9) => Reviewed-by
>
> 3)Singed-off-by (count: 8) => Signed-off-by
>
> 4)Signed-of-by (count: 6) => Signed-off-by
>
> 5)Rewieved-by (count: 3) => Reviewed-by
>
> 6)Signed-off--by (count: 3) => Signed-off-by
>
> 7)Revieved-by (count: 3) => Reviewed-by
>
> 8)Reivewed-by (count: 2) => Reviewed-by
>
> 9)Signef-off-by (count: 2) => Signed-off-by
>
> 10)Test-by (count: 2) => Tested-by
>
> 11)Acked_by (count: 2) => Acked-by
>
> 12)Signed-off-by-by (count: 2) => Signed-off-by
>
> 13)Reported-by-by (count: 1) => Reported-by
>
> 14)Reporetd-by (count: 1) => Reported-by
>
> 15)Reviewed--by (count: 1) => Reviewed-by
>
> 16)Sugested-by (count: 1) => Suggested-by
>
> 17)Suggested--by (count: 1) => Suggested-by
>
> 18)Repoted-by (count: 1) => Reported-by
>
> 19)Rported-by (count: 1) => Reported-by
>
> 20)eigned-off-by (count: 1) => Signed-off-by
>
> 21)Reveiwed-by (count: 1) => Reviewed-by
>
> 22)igned-off-by (count: 1) => Signed-off-by
>
> 23)Tested-by-by (count: 1) => Tested-by
>
> 24)Sugessted-by (count: 1) => Suggested-by
>
> 25)Rewiewed-by (count: 1) => Reviewed-by
>
> 26)Teste-by (count: 1) => Tested-by
>
> 27)Signee-off-by (count: 1) => Signed-off-by
>
> 28)-By (count: 1) => To
>
> 29)Signen-off-by (count: 1) => Signed-off-by
>
> 30)eported-by (count: 1) => Reported-by
>
> 31)Reviewedy-by (count: 1) => Signed-off-by
>
> 32)Siganed-off-by (count: 1) => Signed-off-by
>
> 33)Ackedy-by (count: 1) => Acked-by
>
> 34)Review-by (count: 1) => Reviewed-by
>
> 35)Tweeted-by (count: 1) => Tested-by
>
> 36)Ack-by (count: 1) => Acked-by
>
> 37)Reorted-by (count: 1) => Reported-by
>
> 38)Suggsted-by (count: 1) => Suggested-by
>
> 39)Reviwed-By (count: 1) => Reviewed-by
>

How about summarizing this?

Which are good corrections, which are "bad corrections"; what is the
total count of corrections, how many are good, and which are bad?

How many typo variants were corrected to each valid tag?

> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
> patch applies on next-20201120
>
>  scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..775a49a06179 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +sub get_min {
> +       my (@arr) = @_;
> +       my $len = scalar @arr;
> +       if((scalar @arr) < 1) {
> +               # if underflow, return
> +               return;
> +       }
> +       my $min = $arr[0];
> +       for my $i (0 .. ($len-1)) {
> +               if ($arr[$i] < $min) {
> +                       $min = $arr[$i];
> +               }
> +       }
> +       return $min;
> +}
> +
> +sub get_edit_distance {
> +       my ($str1, $str2) = @_;
> +       my $len1 = length($str1);
> +       my $len2 = length($str2);
> +       # two dimensional array storing minimum edit distance
> +       my @distance;
> +       for my $i (0 .. $len1) {
> +               for my $j (0 .. $len2) {
> +                       if ($i == 0) {
> +                               $distance[$i][$j] = $j;
> +                       }
> +                       elsif ($j == 0) {
> +                               $distance[$i][$j] = $i;
> +                       }
> +                       elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
> +                               $distance[$i][$j] = $distance[$i - 1][$j - 1];
> +                       }
> +                       else {
> +                               my $dist1 = $distance[$i][$j - 1]; #insert distance
> +                               my $dist2 = $distance[$i - 1][$j]; # remove
> +                               my $dist3 = $distance[$i - 1][$j - 1]; #replace
> +                               $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
> +                       }
> +               }
> +       }
> +       return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> +       my ($sign_off) = @_;
> +       $sign_off = lc($sign_off);
> +       $sign_off =~ s/\-//g; # to match with formed hash
> +       my @standard_signature_tags = (
> +               'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
> +               'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
> +       );
> +       # setting default values
> +       my $standard_signature = 'signed-off-by';
> +       my $min_edit_distance = 20;
> +       my $edit_distance;
> +       foreach (@standard_signature_tags) {
> +               my $signature = $_;
> +               $_ =~ s/\-//g;
> +               $edit_distance = get_edit_distance($sign_off, $_);
> +               if ($edit_distance < $min_edit_distance) {
> +                       $min_edit_distance = $edit_distance;
> +                       $standard_signature = $signature;
> +               }
> +       }
> +        if($min_edit_distance<=2) {
> +               return ucfirst($standard_signature);
> +        }
> +       return "";
> +}
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2844,18 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> +                               my $suggested_signature = get_standard_signature($sign_off);
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                       "Non-standard signature: $sign_off\n" . $herecurr);
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                                "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
> +                                           $fix) {
> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.

A large number of these warnings occur because of typo mistakes in
signoffs. An evaluation over v4.13..v5.8 revealed that out of 539
warnings due to Non-standard signatures, 87 are due to typo mistakes.

Following are the standard signature tags which are often incorrectly
used with their counts(over v4.13..v5.8):

1) Reviewed-by => 42
2) Signed-off-by => 25
3) Reported-by => 6
4) Acked-by => 4
5) Tested-by => 4
6) Suggested-by => 4

Provide a fix by calculating levenshtein distance for the signature tag
with all the standard signatures and suggest a fix with signature, whose
edit distance is less than or equal to 2 with the misspelt signature.

Out of the 87 misspelt signatures fixed with this approach, 85 were
found to be good corrections and 2 were bad corrections.

The signature tags which are good corrections using this approach are:

1)Reviwed-by (count: 19) => Reviewed-by
2)Reviewd-by (count: 9) => Reviewed-by
3)Singed-off-by (count: 8) => Signed-off-by
4)Signed-of-by (count: 6) => Signed-off-by
5)Rewieved-by (count: 3) => Reviewed-by
6)Signed-off--by (count: 3) => Signed-off-by
7)Revieved-by (count: 3) => Reviewed-by
8)Reivewed-by (count: 2) => Reviewed-by
9)Signef-off-by (count: 2) => Signed-off-by
10)Test-by (count: 2) => Tested-by
11)Acked_by (count: 2) => Acked-by
12)Signed-off-by-by (count: 2) => Signed-off-by
13)Reported-by-by (count: 1) => Reported-by
14)Reporetd-by (count: 1) => Reported-by
15)Reviewed--by (count: 1) => Reviewed-by
16)Sugested-by (count: 1) => Suggested-by
17)Suggested--by (count: 1) => Suggested-by
18)Repoted-by (count: 1) => Reported-by
19)Rported-by (count: 1) => Reported-by
20)eigned-off-by (count: 1) => Signed-off-by
21)Reveiwed-by (count: 1) => Reviewed-by
22)igned-off-by (count: 1) => Signed-off-by
23)Tested-by-by (count: 1) => Tested-by
24)Sugessted-by (count: 1) => Suggested-by
25)Rewiewed-by (count: 1) => Reviewed-by
26)Teste-by (count: 1) => Tested-by
27)Signee-off-by (count: 1) => Signed-off-by
28)Signen-off-by (count: 1) => Signed-off-by
29)Reviwed-By (count: 1) => Reviewed-by
30)eported-by (count: 1) => Reported-by
31)Reviewedy-by (count: 1) => Reviewed-by
32)Siganed-off-by (count: 1) => Signed-off-by
33)Ackedy-by (count: 1) => Acked-by
34)Review-by (count: 1) => Reviewed-by
35)Suggsted-by (count: 1) => Suggested-by
36)Ack-by (count: 1) => Acked-by
37)Reorted-by (count: 1) => Reported-by

Following were found to be bad corrections:

1)-By (count: 1) => To
2)Tweeted-by (count: 1) => Tested-by

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: modify commit message: replace specific example with overall evaluation, minor changes

changes in v3: summarize commit message

 scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..775a49a06179 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+sub get_min {
+	my (@arr) = @_;
+	my $len = scalar @arr;
+	if((scalar @arr) < 1) {
+		# if underflow, return
+		return;
+	}
+	my $min = $arr[0];
+	for my $i (0 .. ($len-1)) {
+		if ($arr[$i] < $min) {
+			$min = $arr[$i];
+		}
+	}
+	return $min;
+}
+
+sub get_edit_distance {
+	my ($str1, $str2) = @_;
+	my $len1 = length($str1);
+	my $len2 = length($str2);
+	# two dimensional array storing minimum edit distance
+	my @distance;
+	for my $i (0 .. $len1) {
+		for my $j (0 .. $len2) {
+			if ($i == 0) {
+				$distance[$i][$j] = $j;
+			}
+			elsif ($j == 0) {
+				$distance[$i][$j] = $i;
+			}
+			elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+				$distance[$i][$j] = $distance[$i - 1][$j - 1];
+			}
+			else {
+				my $dist1 = $distance[$i][$j - 1]; #insert distance
+				my $dist2 = $distance[$i - 1][$j]; # remove
+				my $dist3 = $distance[$i - 1][$j - 1]; #replace
+				$distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+			}
+		}
+	}
+	return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+	my ($sign_off) = @_;
+	$sign_off = lc($sign_off);
+	$sign_off =~ s/\-//g; # to match with formed hash
+	my @standard_signature_tags = (
+		'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+		'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
+	);
+	# setting default values
+	my $standard_signature = 'signed-off-by';
+	my $min_edit_distance = 20;
+	my $edit_distance;
+	foreach (@standard_signature_tags) {
+		my $signature = $_;
+		$_ =~ s/\-//g;
+		$edit_distance = get_edit_distance($sign_off, $_);
+		if ($edit_distance < $min_edit_distance) {
+			$min_edit_distance = $edit_distance;
+			$standard_signature = $signature;
+		}
+	}
+        if($min_edit_distance<=2) {
+		return ucfirst($standard_signature);
+        }
+	return "";
+}
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2844,18 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = get_standard_signature($sign_off);
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					"Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						 "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
+					    $fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
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 v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Wed, Nov 25, 2020 at 12:25 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signoffs. An evaluation over v4.13..v5.8 revealed that out of 539

It is not a sign-off, it is a signature tag.

> warnings due to Non-standard signatures, 87 are due to typo mistakes.
>
why do you write "non-standard" capitalized?

> Following are the standard signature tags which are often incorrectly
> used with their counts(over v4.13..v5.8):
>

This sentence above is not clear.

> 1) Reviewed-by => 42
> 2) Signed-off-by => 25
> 3) Reported-by => 6
> 4) Acked-by => 4
> 5) Tested-by => 4
> 6) Suggested-by => 4
>

You can probably drop the 1) to 6); simply indent it; I think "tag:
count" or "tag     count" (where the spacing is aligned) is better
than "tag => count"

> Provide a fix by calculating levenshtein distance for the signature tag
> with all the standard signatures and suggest a fix with signature, whose
> edit distance is less than or equal to 2 with the misspelt signature.
>

s/misspelt/misspelled/

> Out of the 87 misspelt signatures fixed with this approach, 85 were
> found to be good corrections and 2 were bad corrections.
>
> The signature tags which are good corrections using this approach are:
>
> 1)Reviwed-by (count: 19) => Reviewed-by
> 2)Reviewd-by (count: 9) => Reviewed-by
> 3)Singed-off-by (count: 8) => Signed-off-by
> 4)Signed-of-by (count: 6) => Signed-off-by
> 5)Rewieved-by (count: 3) => Reviewed-by
> 6)Signed-off--by (count: 3) => Signed-off-by
> 7)Revieved-by (count: 3) => Reviewed-by
> 8)Reivewed-by (count: 2) => Reviewed-by
> 9)Signef-off-by (count: 2) => Signed-off-by
> 10)Test-by (count: 2) => Tested-by
> 11)Acked_by (count: 2) => Acked-by
> 12)Signed-off-by-by (count: 2) => Signed-off-by
> 13)Reported-by-by (count: 1) => Reported-by
> 14)Reporetd-by (count: 1) => Reported-by
> 15)Reviewed--by (count: 1) => Reviewed-by
> 16)Sugested-by (count: 1) => Suggested-by
> 17)Suggested--by (count: 1) => Suggested-by
> 18)Repoted-by (count: 1) => Reported-by
> 19)Rported-by (count: 1) => Reported-by
> 20)eigned-off-by (count: 1) => Signed-off-by
> 21)Reveiwed-by (count: 1) => Reviewed-by
> 22)igned-off-by (count: 1) => Signed-off-by
> 23)Tested-by-by (count: 1) => Tested-by
> 24)Sugessted-by (count: 1) => Suggested-by
> 25)Rewiewed-by (count: 1) => Reviewed-by
> 26)Teste-by (count: 1) => Tested-by
> 27)Signee-off-by (count: 1) => Signed-off-by
> 28)Signen-off-by (count: 1) => Signed-off-by
> 29)Reviwed-By (count: 1) => Reviewed-by
> 30)eported-by (count: 1) => Reported-by
> 31)Reviewedy-by (count: 1) => Reviewed-by
> 32)Siganed-off-by (count: 1) => Signed-off-by
> 33)Ackedy-by (count: 1) => Acked-by
> 34)Review-by (count: 1) => Reviewed-by
> 35)Suggsted-by (count: 1) => Suggested-by
> 36)Ack-by (count: 1) => Acked-by
> 37)Reorted-by (count: 1) => Reported-by
>

I do not think the enumeration of all those typos is interesting for
the commit message. Maybe Joe will ask, but anyone else can imagine
how such typos look like.

If you think they should be kept, I would just sort it this way:

Typo variants of the tags were:

Signed-off-by:
  then list all variants

Acked-by:
  then list all variants

Reported-by:
  then list all variants

etc.

> Following were found to be bad corrections:
>
> 1)-By (count: 1) => To

How about not correcting to To: (It is simply too short).

The explanation is getting better, give it another try. Then we might
be in a good state for Joe and lkml.

Lukas

> 2)Tweeted-by (count: 1) => Tested-by
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
>
> changes in v3: summarize commit message
>
>  scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..775a49a06179 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +sub get_min {
> +       my (@arr) = @_;
> +       my $len = scalar @arr;
> +       if((scalar @arr) < 1) {
> +               # if underflow, return
> +               return;
> +       }
> +       my $min = $arr[0];
> +       for my $i (0 .. ($len-1)) {
> +               if ($arr[$i] < $min) {
> +                       $min = $arr[$i];
> +               }
> +       }
> +       return $min;
> +}
> +
> +sub get_edit_distance {
> +       my ($str1, $str2) = @_;
> +       my $len1 = length($str1);
> +       my $len2 = length($str2);
> +       # two dimensional array storing minimum edit distance
> +       my @distance;
> +       for my $i (0 .. $len1) {
> +               for my $j (0 .. $len2) {
> +                       if ($i == 0) {
> +                               $distance[$i][$j] = $j;
> +                       }
> +                       elsif ($j == 0) {
> +                               $distance[$i][$j] = $i;
> +                       }
> +                       elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
> +                               $distance[$i][$j] = $distance[$i - 1][$j - 1];
> +                       }
> +                       else {
> +                               my $dist1 = $distance[$i][$j - 1]; #insert distance
> +                               my $dist2 = $distance[$i - 1][$j]; # remove
> +                               my $dist3 = $distance[$i - 1][$j - 1]; #replace
> +                               $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
> +                       }
> +               }
> +       }
> +       return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> +       my ($sign_off) = @_;
> +       $sign_off = lc($sign_off);
> +       $sign_off =~ s/\-//g; # to match with formed hash
> +       my @standard_signature_tags = (
> +               'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
> +               'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
> +       );
> +       # setting default values
> +       my $standard_signature = 'signed-off-by';
> +       my $min_edit_distance = 20;
> +       my $edit_distance;
> +       foreach (@standard_signature_tags) {
> +               my $signature = $_;
> +               $_ =~ s/\-//g;
> +               $edit_distance = get_edit_distance($sign_off, $_);
> +               if ($edit_distance < $min_edit_distance) {
> +                       $min_edit_distance = $edit_distance;
> +                       $standard_signature = $signature;
> +               }
> +       }
> +        if($min_edit_distance<=2) {
> +               return ucfirst($standard_signature);
> +        }
> +       return "";
> +}
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2844,18 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> +                               my $suggested_signature = get_standard_signature($sign_off);
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                       "Non-standard signature: $sign_off\n" . $herecurr);
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                                "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
> +                                           $fix) {
> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v4] checkpatch: add fix and improve warning msg for non-standard signature

[Aditya Srivastava]

Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.

A large number of these warnings occur because of typo mistakes in
signature tags. An evaluation over v4.13..v5.8 showed that out of 539
warnings due to non-standard signatures, 87 are due to typo mistakes.

Following are the standard signature tags which are often incorrectly
used, along with their individual counts of incorrect use (over
v4.13..v5.8):

 Reviewed-by: 42
 Signed-off-by: 25
 Reported-by: 6
 Acked-by: 4
 Tested-by: 4
 Suggested-by: 4

Provide a fix by calculating levenshtein distance for the signature tag
with all the standard signatures and suggest a fix with a signature, whose
edit distance is less than or equal to 2 with the misspelled signature.

Out of the 86 mispelled signatures fixed with this approach, 85 were
found to be good corrections and 1 was bad correction.

Following was found to be a bad correction:
 Tweeted-by (count: 1) => Tested-by

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: modify commit message: replace specific example with overall evaluation, minor changes

changes in v3: summarize commit message

changes in v4: improve commit message; remove signature suggestions of small length (ie 'cc' and 'to')

 scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..2b1afd763d8d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+sub get_min {
+	my (@arr) = @_;
+	my $len = scalar @arr;
+	if((scalar @arr) < 1) {
+		# if underflow, return
+		return;
+	}
+	my $min = $arr[0];
+	for my $i (0 .. ($len-1)) {
+		if ($arr[$i] < $min) {
+			$min = $arr[$i];
+		}
+	}
+	return $min;
+}
+
+sub get_edit_distance {
+	my ($str1, $str2) = @_;
+	my $len1 = length($str1);
+	my $len2 = length($str2);
+	# two dimensional array storing minimum edit distance
+	my @distance;
+	for my $i (0 .. $len1) {
+		for my $j (0 .. $len2) {
+			if ($i == 0) {
+				$distance[$i][$j] = $j;
+			}
+			elsif ($j == 0) {
+				$distance[$i][$j] = $i;
+			}
+			elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+				$distance[$i][$j] = $distance[$i - 1][$j - 1];
+			}
+			else {
+				my $dist1 = $distance[$i][$j - 1]; #insert distance
+				my $dist2 = $distance[$i - 1][$j]; # remove
+				my $dist3 = $distance[$i - 1][$j - 1]; #replace
+				$distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+			}
+		}
+	}
+	return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+	my ($sign_off) = @_;
+	$sign_off = lc($sign_off);
+	$sign_off =~ s/\-//g; # to match with formed hash
+	my @standard_signature_tags = (
+		'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+		'reviewed-by:', 'reported-by:', 'suggested-by:'
+	);
+	# setting default values
+	my $standard_signature = 'signed-off-by';
+	my $min_edit_distance = 20;
+	my $edit_distance;
+	foreach (@standard_signature_tags) {
+		my $signature = $_;
+		$_ =~ s/\-//g;
+		$edit_distance = get_edit_distance($sign_off, $_);
+		if ($edit_distance < $min_edit_distance) {
+			$min_edit_distance = $edit_distance;
+			$standard_signature = $signature;
+		}
+	}
+        if($min_edit_distance<=2) {
+		return ucfirst($standard_signature);
+        }
+	return "";
+}
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2844,18 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = get_standard_signature($sign_off);
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					"Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						 "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
+					    $fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
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 v4] checkpatch: add fix and improve warning msg for non-standard signature

[Lukas Bulwahn]

[-- Attachment #1.1: Type: text/plain, Size: 6014 bytes --]

Aditya Srivastava <yashsri421@gmail.com> schrieb am Sa., 28. Nov. 2020 um
10:52:

> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signature tags. An evaluation over v4.13..v5.8 showed that out of 539
> warnings due to non-standard signatures, 87 are due to typo mistakes.
>
> Following are the standard signature tags which are often incorrectly
> used, along with their individual counts of incorrect use (over
> v4.13..v5.8):
>
>  Reviewed-by: 42
>  Signed-off-by: 25
>  Reported-by: 6
>  Acked-by: 4
>  Tested-by: 4
>  Suggested-by: 4
>
> Provide a fix by calculating levenshtein distance for the signature tag
> with all the standard signatures and suggest a fix with a signature, whose
> edit distance is less than or equal to 2 with the misspelled signature.
>
> Out of the 86 mispelled signatures fixed with this approach, 85 were
> found to be good corrections and 1 was bad correction.
>
> Following was found to be a bad correction:
>  Tweeted-by (count: 1) => Tested-by
>

Looks Good. Let us send it to Joe.

>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: modify commit message: replace specific example with
> overall evaluation, minor changes
>
> changes in v3: summarize commit message
>
> changes in v4: improve commit message; remove signature suggestions of
> small length (ie 'cc' and 'to')
>
>  scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..2b1afd763d8d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +sub get_min {
> +       my (@arr) = @_;
> +       my $len = scalar @arr;
> +       if((scalar @arr) < 1) {
> +               # if underflow, return
> +               return;
> +       }
> +       my $min = $arr[0];
> +       for my $i (0 .. ($len-1)) {
> +               if ($arr[$i] < $min) {
> +                       $min = $arr[$i];
> +               }
> +       }
> +       return $min;
> +}
> +
> +sub get_edit_distance {
> +       my ($str1, $str2) = @_;
> +       my $len1 = length($str1);
> +       my $len2 = length($str2);
> +       # two dimensional array storing minimum edit distance
> +       my @distance;
> +       for my $i (0 .. $len1) {
> +               for my $j (0 .. $len2) {
> +                       if ($i == 0) {
> +                               $distance[$i][$j] = $j;
> +                       }
> +                       elsif ($j == 0) {
> +                               $distance[$i][$j] = $i;
> +                       }
> +                       elsif (substr($str1, $i-1, 1) eq substr($str2,
> $j-1, 1)) {
> +                               $distance[$i][$j] = $distance[$i - 1][$j -
> 1];
> +                       }
> +                       else {
> +                               my $dist1 = $distance[$i][$j - 1]; #insert
> distance
> +                               my $dist2 = $distance[$i - 1][$j]; # remove
> +                               my $dist3 = $distance[$i - 1][$j - 1];
> #replace
> +                               $distance[$i][$j] = 1 + get_min($dist1,
> $dist2, $dist3);
> +                       }
> +               }
> +       }
> +       return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> +       my ($sign_off) = @_;
> +       $sign_off = lc($sign_off);
> +       $sign_off =~ s/\-//g; # to match with formed hash
> +       my @standard_signature_tags = (
> +               'signed-off-by:', 'co-developed-by:', 'acked-by:',
> 'tested-by:',
> +               'reviewed-by:', 'reported-by:', 'suggested-by:'
> +       );
> +       # setting default values
> +       my $standard_signature = 'signed-off-by';
> +       my $min_edit_distance = 20;
> +       my $edit_distance;
> +       foreach (@standard_signature_tags) {
> +               my $signature = $_;
> +               $_ =~ s/\-//g;
> +               $edit_distance = get_edit_distance($sign_off, $_);
> +               if ($edit_distance < $min_edit_distance) {
> +                       $min_edit_distance = $edit_distance;
> +                       $standard_signature = $signature;
> +               }
> +       }
> +        if($min_edit_distance<=2) {
> +               return ucfirst($standard_signature);
> +        }
> +       return "";
> +}
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2844,18 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n"
> . $herecurr);
> +                               my $suggested_signature =
> get_standard_signature($sign_off);
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                       "Non-standard signature:
> $sign_off\n" . $herecurr);
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                                "Non-standard signature:
> $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
> +                                           $fix) {
> +                                               $fixed[$fixlinenr] =~
> s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 2.17.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 8294 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

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

[Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
styles.

This warning occurs because of incorrect use of signature tags,
e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
signature tags, which may seem correct, but are not standard:

1) Requested-by (count: 48) => Suggested-by
Rationale: In an open-source project, there are no 'requests', just
'suggestions' to convince a maintainer to accept your patch

2) Co-authored-by (count: 43) => Co-developed-by
Rationale: Co-developed-by and Co-authored-by are synonyms

3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
Rationale: Analyzing is a part of Software Development, so
'Co-developed-by' is perfectly fine, even if contributor did not create
code

4) Improvements-by (count: 19) => Co-developed-by

5) Noticed-by (count: 11) => Reported-by

6) Inspired-by (count: 11) => Suggested-by

7) Verified-by (count: 8) => Tested-by
Rationale: Used by a single user. On reading mailing list, it seems
Tested-by might be a suitable alternative

8) Okay-ished-by (count: 8) => Acked-by
Rationale: Used by a single user. On reading mailing list, it seems
Acked-by must be suitable alternative

9) Acked-for-MFD-by (count: 6) => Acked-by

10) Reviewed-off-by (count: 5) => Reviewed-by

11) Proposed-by (count: 5) => Suggested-by
Rationale: On observing the mailing list, this tag is always used for a
maintainer. It seems that the changes might have been suggested by them
and the tag is used as acknowledgment for the same

12) Fixed-by (count: 3) => Co-developed-by
Rationale: Fixing bug is a part of Software Development, so
'Co-developed-by' is perfectly fine, even if contributor did not create
code

13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
Rationale: The tags are used for maintainers. It seems that the changes
might have been suggested by them and the tag is used as acknowledgment
for the same
E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

14) Suggestions-by (count: 3) => Suggested-by

15) Generated-by (count: 17) => remove the tag
On observing the mailing list, this tag is always used for quoting the
tool or script, which might have been used to generate the patch.
E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci

16) Celebrated-by (count: 3) => remove the tag
This tag was used for only one commit. On observing mailing list, it seem
like the celebration for a particular patch and changes.

Provide a fix by:
1) replacing the non-standard signature with its standard equivalent
2) removing the signature if it is not required

Also, improve warning messages correspondingly, providing users
suggestions to either replace or remove the signature. Also provide
suitable rationale to the user for the suggestion made.

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: replace commit specific example with brief evaluation

changes in v3: provide rationale to users for every signature tag suggestion;
modify commit message describing arrival to conclusion in a structured way

 scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 99 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..f402c9c3958f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+our %standard_signature_fix = (
+	"Requested-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
+	},
+	"Co-authored-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Co-developed-by and Co-authored-by are synonyms",
+	},
+	"Analyzed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Analysed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Improvements-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Performing improvements are a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Noticed-by:" => {
+		suggestion => "Reported-by:",
+		rationale => "Reported-by and Noticed-by are synonyms",
+	},
+	"Inspired-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
+	},
+	"Verified-by:" => {
+		suggestion => "Tested-by:",
+		rationale => "Tested-by and Verified-by are synonyms",
+	},
+	"Okay-ished-by:" => {
+		suggestion => "Acked-by:",
+		rationale => "Acked-by is the standard signature tag for recording your approval",
+	},
+	"Acked-for-MFD-by:" => {
+		suggestion => "Acked-by:",
+		rationale => "Acked-by is the standard signature tag for recording your approval",
+	},
+	"Reviewed-off-by:" => {
+		suggestion => "Reviewed-by:",
+		rationale => "Reviewed-by is the standard signature tag for recording your approval",
+	},
+	"Proposed-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Fixed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Pointed-out-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Pointed-at-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Suggestions-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
+	},
+	"Generated-by:" => {
+		suggestion => "remove",
+		rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
+	},
+	"Celebrated-by:" => {
+		suggestion => "remove",
+		rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
+	},
+);
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2848,30 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = "";
+				my $rationale = "";
+				if (exists($standard_signature_fix{$sign_off})) {
+					$suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
+					$rationale = $standard_signature_fix{$sign_off}{'rationale'};
+				}
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					     "Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				elsif ($suggested_signature eq "remove") {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
+					$fix) {
+						fix_delete_line($fixlinenr, $rawline);
+					}
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
+					$fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
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 v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Mon, Nov 23, 2020 at 4:04 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> This warning occurs because of incorrect use of signature tags,
> e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
> signature tags, which may seem correct, but are not standard:
>
> 1) Requested-by (count: 48) => Suggested-by
> Rationale: In an open-source project, there are no 'requests', just
> 'suggestions' to convince a maintainer to accept your patch
>
> 2) Co-authored-by (count: 43) => Co-developed-by
> Rationale: Co-developed-by and Co-authored-by are synonyms
>
> 3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
> Rationale: Analyzing is a part of Software Development, so
> 'Co-developed-by' is perfectly fine, even if contributor did not create
> code
>
> 4) Improvements-by (count: 19) => Co-developed-by
>
> 5) Noticed-by (count: 11) => Reported-by
>
> 6) Inspired-by (count: 11) => Suggested-by
>
> 7) Verified-by (count: 8) => Tested-by
> Rationale: Used by a single user. On reading mailing list, it seems
> Tested-by might be a suitable alternative
>
> 8) Okay-ished-by (count: 8) => Acked-by
> Rationale: Used by a single user. On reading mailing list, it seems
> Acked-by must be suitable alternative
>
> 9) Acked-for-MFD-by (count: 6) => Acked-by
>
> 10) Reviewed-off-by (count: 5) => Reviewed-by
>
> 11) Proposed-by (count: 5) => Suggested-by
> Rationale: On observing the mailing list, this tag is always used for a
> maintainer. It seems that the changes might have been suggested by them
> and the tag is used as acknowledgment for the same
>
> 12) Fixed-by (count: 3) => Co-developed-by
> Rationale: Fixing bug is a part of Software Development, so
> 'Co-developed-by' is perfectly fine, even if contributor did not create
> code
>
> 13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
> Rationale: The tags are used for maintainers. It seems that the changes
> might have been suggested by them and the tag is used as acknowledgment
> for the same
> E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> 14) Suggestions-by (count: 3) => Suggested-by
>
> 15) Generated-by (count: 17) => remove the tag
> On observing the mailing list, this tag is always used for quoting the
> tool or script, which might have been used to generate the patch.
> E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
> 16) Celebrated-by (count: 3) => remove the tag
> This tag was used for only one commit. On observing mailing list, it seem
> like the celebration for a particular patch and changes.
>
> Provide a fix by:
> 1) replacing the non-standard signature with its standard equivalent
> 2) removing the signature if it is not required
>
> Also, improve warning messages correspondingly, providing users
> suggestions to either replace or remove the signature. Also provide
> suitable rationale to the user for the suggestion made.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: replace commit specific example with brief evaluation
>
> changes in v3: provide rationale to users for every signature tag suggestion;
> modify commit message describing arrival to conclusion in a structured way
>
>  scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..f402c9c3958f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +our %standard_signature_fix = (
> +       "Requested-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
> +       },
> +       "Co-authored-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Co-developed-by and Co-authored-by are synonyms",
> +       },

Maybe this reads better:
Co-developed-by is the standard tag for patches developed by multiple authors.

> +       "Analyzed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Analysed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Improvements-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Performing improvements are a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Noticed-by:" => {
> +               suggestion => "Reported-by:",
> +               rationale => "Reported-by and Noticed-by are synonyms",
> +       },

Similarly:

Reported-by is the standard tag for denoting any notice or report.

> +       "Inspired-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> +       },
> +       "Verified-by:" => {
> +               suggestion => "Tested-by:",
> +               rationale => "Tested-by and Verified-by are synonyms",
> +       },

Similarly here.

> +       "Okay-ished-by:" => {
> +               suggestion => "Acked-by:",
> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> +       },

I think "your" is wrong here; the person that runs checkpatch may not
be the person providing the approval.

So maybe:

- for recording one's approval.

or:
- for recording acknowledgement.

> +       "Acked-for-MFD-by:" => {
> +               suggestion => "Acked-by:",
> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> +       },
> +       "Reviewed-off-by:" => {
> +               suggestion => "Reviewed-by:",
> +               rationale => "Reviewed-by is the standard signature tag for recording your approval",
> +       },

for recording the review.

> +       "Proposed-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
> +       },

Similarly, the sentence as above could be used.

> +       "Fixed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Pointed-out-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Pointed-at-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Suggestions-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> +       },
> +       "Generated-by:" => {
> +               suggestion => "remove",
> +               rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
> +       },
> +       "Celebrated-by:" => {
> +               suggestion => "remove",
> +               rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
> +       },
> +);
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2848,30 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> +                               my $suggested_signature = "";
> +                               my $rationale = "";
> +                               if (exists($standard_signature_fix{$sign_off})) {
> +                                       $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
> +                                       $rationale = $standard_signature_fix{$sign_off}{'rationale'};
> +                               }
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
> +                               }
> +                               elsif ($suggested_signature eq "remove") {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
> +                                       $fix) {
> +                                               fix_delete_line($fixlinenr, $rawline);
> +                                       }
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
> +                                       $fix) {
> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 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: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Sat, Nov 21, 2020 at 5:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Checkpatch.pl warns on non-standard signature styles.
>
> This warning usually occurs because of incorrect use of signature tags,
> e.g. tags such as 'Co-authored-by', which although seem correct, is not
> a standard signature. Its standard equivalent is 'Co-developed-by'.
>
> An evaluation over v4.13..v5.8 found 'Co-authored-by' tag used 43 times
>
> Similarly, 'Requested-by'(used 48 times) has its equivalent as
> 'Suggested-by', and so on.
>

Better but still an incomplete summary.
"and so on" is not good.

Consider a description that explains how you came to the conclusion.

Then consider presenting the count of non-standard signature tags from
v4.13..v5.8 is a simple structured form.

By the way, there is no limit on the length of a commit message as
long as you convey information in a dense form.

Also, consider to present the rationale for each replacement.
checkpatch shall explain to its user why it considers the replacement
appropriate.


Lukas

> Provide a fix by:
> 1) replacing the non-standard signature with its standard equivalent
> 2) removing the signature if it is not required
>
> Also, improve warning messages correspondingly, providing users
> suggestions to either replace or remove the signature
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: replace commit specific example with brief evaluation
>
>  scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..23a21dc2c29a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +our %standard_signature_fix = (
> +       "Requested-by:" => "Suggested-by:",
> +       "Co-authored-by:" => "Co-developed-by:",
> +       "Analyzed-by:" => "Co-developed-by:",
> +       "Analysed-by:" => "Co-developed-by:",
> +       "Improvements-by:" => "Co-developed-by:",
> +       "Noticed-by:" => "Reported-by:",
> +       "Inspired-by:" => "Suggested-by:",
> +       "Verified-by:" => "Tested-by:",
> +       "Okay-ished-by:" => "Acked-by:",
> +       "Acked-for-MFD-by:" => "Acked-by:",
> +       "Reviewed-off-by:" => "Reviewed-by:",
> +       "Proposed-by:" => "Suggested-by:",
> +       "Fixed-by:" => "Co-developed-by:",
> +       "Pointed-out-by:" => "Suggested-by:",
> +       "Pointed-at-by:" => "Suggested-by:",
> +       "Suggestions-by:" => "Suggested-by:",
> +       "Generated-by:" => "remove",
> +       "Celebrated-by:" => "remove",
> +);
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2794,28 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> +                               my $suggested_signature = "";
> +                               if (exists($standard_signature_fix{$sign_off})) {
> +                                       $suggested_signature = $standard_signature_fix{$sign_off};
> +                               }
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
> +                               }
> +                               elsif ($suggested_signature eq "remove") {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
> +                                       $fix) {
> +                                               fix_delete_line($fixlinenr, $rawline);
> +                                       }
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
> +                                       $fix) {
> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
styles.

This warning occurs because of incorrect use of signature tags,
e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
signature tags, which may seem correct, but are not standard:

1) Requested-by (count: 48) => Suggested-by
Rationale: In an open-source project, there are no 'requests', just
'suggestions' to convince a maintainer to accept your patch

2) Co-authored-by (count: 43) => Co-developed-by
Rationale: Co-developed-by and Co-authored-by are synonyms

3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
Rationale: Analyzing is a part of Software Development, so
'Co-developed-by' is perfectly fine, even if contributor did not create
code

4) Improvements-by (count: 19) => Co-developed-by

5) Noticed-by (count: 11) => Reported-by

6) Inspired-by (count: 11) => Suggested-by

7) Verified-by (count: 8) => Tested-by
Rationale: Used by a single user. On reading mailing list, it seems
Tested-by might be a suitable alternative

8) Okay-ished-by (count: 8) => Acked-by
Rationale: Used by a single user. On reading mailing list, it seems
Acked-by must be suitable alternative

9) Acked-for-MFD-by (count: 6) => Acked-by

10) Reviewed-off-by (count: 5) => Reviewed-by

11) Proposed-by (count: 5) => Suggested-by
Rationale: On observing the mailing list, this tag is always used for a
maintainer. It seems that the changes might have been suggested by them
and the tag is used as acknowledgement for the same

12) Fixed-by (count: 3) => Co-developed-by
Rationale: Fixing bug is a part of Software Development, so
'Co-developed-by' is perfectly fine, even if contributor did not create
code

13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
Rationale: The tags are used for maintainers. It seems that the changes
might have been suggested by them and the tag is used as acknowledgement
for the same
E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

14) Suggestions-by (count: 3) => Suggested-by

15) Generated-by (count: 17) => remove the tag
On observing the mailing list, this tag is always used for quoting the
tool or script, which might have been used to generate the patch.
E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci

16) Celebrated-by (count: 3) => remove the tag
This tag was used for only one commit. On observing mailing list, it seem
like the celebration for a particular patch and changes.

Provide a fix by:
1) replacing the non-standard signature with its standard equivalent
2) removing the signature if it is not required

Also, improve warning messages correspondingly, providing users
suggestions to either replace or remove the signature. Also provide
suitable rationale to the user for the suggestion made.

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: replace commit specific example with brief evaluation

changes in v3: provide rationale to users for every signature tag suggestion;
modify commit message describing arrival to conclusion in a structured way

 scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 99 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..575ff8efb0eb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+our %standard_signature_fix = (
+	"Requested-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
+	},
+	"Co-authored-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Co-developed-by and Co-authored-by are synonyms",
+	},
+	"Analyzed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Analysed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Improvements-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Noticed-by:" => {
+		suggestion => "Reported-by:",
+		rationale => "Reported-by and Noticed-by are synonyms",
+	},
+	"Inspired-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
+	},
+	"Verified-by:" => {
+		suggestion => "Tested-by:",
+		rationale => "Tested-by and Verified-by are synonyms",
+	},
+	"Okay-ished-by:" => {
+		suggestion => "Acked-by:",
+		rationale => "Acked-by is the standard signature tag for recording your approval",
+	},
+	"Acked-for-MFD-by:" => {
+		suggestion => "Acked-by:",
+		rationale => "Acked-by is the standard signature tag for recording your approval",
+	},
+	"Reviewed-off-by:" => {
+		suggestion => "Reviewed-by:",
+		rationale => "Reviewed-by is the standard signature tag for recording your approval",
+	},
+	"Proposed-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Fixed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Pointed-out-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Pointed-at-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Suggestions-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
+	},
+	"Generated-by:" => {
+		suggestion => "remove",
+		rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
+	},
+	"Celebrated-by:" => {
+		suggestion => "remove",
+		rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
+	},
+);
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2848,30 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = "";
+				my $rationale = "";
+				if (exists($standard_signature_fix{$sign_off})) {
+					$suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
+					$rationale = $standard_signature_fix{$sign_off}{'rationale'};
+				}
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					     "Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				elsif ($suggested_signature eq "remove") {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
+					$fix) {
+						fix_delete_line($fixlinenr, $rawline);
+					}
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
+					$fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
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 v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Mon, Nov 23, 2020 at 1:21 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> This warning occurs because of incorrect use of signature tags,
> e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
> signature tags, which may seem correct, but are not standard:
>
> 1) Requested-by (count: 48) => Suggested-by
> Rationale: In an open-source project, there are no 'requests', just
> 'suggestions' to convince a maintainer to accept your patch
>
> 2) Co-authored-by (count: 43) => Co-developed-by
> Rationale: Co-developed-by and Co-authored-by are synonyms
>
> 3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
> Rationale: Analyzing is a part of Software Development, so
> 'Co-developed-by' is perfectly fine, even if contributor did not create
> code
>
> 4) Improvements-by (count: 19) => Co-developed-by
>
> 5) Noticed-by (count: 11) => Reported-by
>
> 6) Inspired-by (count: 11) => Suggested-by
>
> 7) Verified-by (count: 8) => Tested-by
> Rationale: Used by a single user. On reading mailing list, it seems
> Tested-by might be a suitable alternative
>
> 8) Okay-ished-by (count: 8) => Acked-by
> Rationale: Used by a single user. On reading mailing list, it seems
> Acked-by must be suitable alternative
>
> 9) Acked-for-MFD-by (count: 6) => Acked-by
>
> 10) Reviewed-off-by (count: 5) => Reviewed-by
>
> 11) Proposed-by (count: 5) => Suggested-by
> Rationale: On observing the mailing list, this tag is always used for a
> maintainer. It seems that the changes might have been suggested by them
> and the tag is used as acknowledgement for the same
>
> 12) Fixed-by (count: 3) => Co-developed-by
> Rationale: Fixing bug is a part of Software Development, so
> 'Co-developed-by' is perfectly fine, even if contributor did not create
> code
>
> 13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
> Rationale: The tags are used for maintainers. It seems that the changes
> might have been suggested by them and the tag is used as acknowledgement
> for the same
> E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> 14) Suggestions-by (count: 3) => Suggested-by
>
> 15) Generated-by (count: 17) => remove the tag
> On observing the mailing list, this tag is always used for quoting the
> tool or script, which might have been used to generate the patch.
> E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
> 16) Celebrated-by (count: 3) => remove the tag
> This tag was used for only one commit. On observing mailing list, it seem
> like the celebration for a particular patch and changes.
>
> Provide a fix by:
> 1) replacing the non-standard signature with its standard equivalent
> 2) removing the signature if it is not required
>
> Also, improve warning messages correspondingly, providing users
> suggestions to either replace or remove the signature. Also provide
> suitable rationale to the user for the suggestion made.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>


Looks good to me. Let us propose this to Joe for review.

Also, you can already start working on the related feature for
providing fixes based on the edit distance.

With both features, we can probably fix all non-standard signatures...

Lukas

> ---
> changes in v2: replace commit specific example with brief evaluation
>
> changes in v3: provide rationale to users for every signature tag suggestion;
> modify commit message describing arrival to conclusion in a structured way
>
>  scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..575ff8efb0eb 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +our %standard_signature_fix = (
> +       "Requested-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
> +       },
> +       "Co-authored-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Co-developed-by and Co-authored-by are synonyms",
> +       },
> +       "Analyzed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Analysed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Improvements-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Noticed-by:" => {
> +               suggestion => "Reported-by:",
> +               rationale => "Reported-by and Noticed-by are synonyms",
> +       },
> +       "Inspired-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> +       },
> +       "Verified-by:" => {
> +               suggestion => "Tested-by:",
> +               rationale => "Tested-by and Verified-by are synonyms",
> +       },
> +       "Okay-ished-by:" => {
> +               suggestion => "Acked-by:",
> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> +       },
> +       "Acked-for-MFD-by:" => {
> +               suggestion => "Acked-by:",
> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> +       },
> +       "Reviewed-off-by:" => {
> +               suggestion => "Reviewed-by:",
> +               rationale => "Reviewed-by is the standard signature tag for recording your approval",
> +       },
> +       "Proposed-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Fixed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Pointed-out-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Pointed-at-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Suggestions-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> +       },
> +       "Generated-by:" => {
> +               suggestion => "remove",
> +               rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
> +       },
> +       "Celebrated-by:" => {
> +               suggestion => "remove",
> +               rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
> +       },
> +);
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2848,30 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> +                               my $suggested_signature = "";
> +                               my $rationale = "";
> +                               if (exists($standard_signature_fix{$sign_off})) {
> +                                       $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
> +                                       $rationale = $standard_signature_fix{$sign_off}{'rationale'};
> +                               }
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
> +                               }
> +                               elsif ($suggested_signature eq "remove") {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
> +                                       $fix) {
> +                                               fix_delete_line($fixlinenr, $rawline);
> +                                       }
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
> +                                       $fix) {
> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 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 v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya]

On 23/11/20 6:39 pm, Lukas Bulwahn wrote:
> On Mon, Nov 23, 2020 at 1:21 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>
>> Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
>> styles.
>>
>> This warning occurs because of incorrect use of signature tags,
>> e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
>> signature tags, which may seem correct, but are not standard:
>>
>> 1) Requested-by (count: 48) => Suggested-by
>> Rationale: In an open-source project, there are no 'requests', just
>> 'suggestions' to convince a maintainer to accept your patch
>>
>> 2) Co-authored-by (count: 43) => Co-developed-by
>> Rationale: Co-developed-by and Co-authored-by are synonyms
>>
>> 3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
>> Rationale: Analyzing is a part of Software Development, so
>> 'Co-developed-by' is perfectly fine, even if contributor did not create
>> code
>>
>> 4) Improvements-by (count: 19) => Co-developed-by
>>
>> 5) Noticed-by (count: 11) => Reported-by
>>
>> 6) Inspired-by (count: 11) => Suggested-by
>>
>> 7) Verified-by (count: 8) => Tested-by
>> Rationale: Used by a single user. On reading mailing list, it seems
>> Tested-by might be a suitable alternative
>>
>> 8) Okay-ished-by (count: 8) => Acked-by
>> Rationale: Used by a single user. On reading mailing list, it seems
>> Acked-by must be suitable alternative
>>
>> 9) Acked-for-MFD-by (count: 6) => Acked-by
>>
>> 10) Reviewed-off-by (count: 5) => Reviewed-by
>>
>> 11) Proposed-by (count: 5) => Suggested-by
>> Rationale: On observing the mailing list, this tag is always used for a
>> maintainer. It seems that the changes might have been suggested by them
>> and the tag is used as acknowledgement for the same
>>
>> 12) Fixed-by (count: 3) => Co-developed-by
>> Rationale: Fixing bug is a part of Software Development, so
>> 'Co-developed-by' is perfectly fine, even if contributor did not create
>> code
>>
>> 13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
>> Rationale: The tags are used for maintainers. It seems that the changes
>> might have been suggested by them and the tag is used as acknowledgement
>> for the same
>> E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>> 14) Suggestions-by (count: 3) => Suggested-by
>>
>> 15) Generated-by (count: 17) => remove the tag
>> On observing the mailing list, this tag is always used for quoting the
>> tool or script, which might have been used to generate the patch.
>> E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>>
>> 16) Celebrated-by (count: 3) => remove the tag
>> This tag was used for only one commit. On observing mailing list, it seem
>> like the celebration for a particular patch and changes.
>>
>> Provide a fix by:
>> 1) replacing the non-standard signature with its standard equivalent
>> 2) removing the signature if it is not required
>>
>> Also, improve warning messages correspondingly, providing users
>> suggestions to either replace or remove the signature. Also provide
>> suitable rationale to the user for the suggestion made.
>>
>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> 
> 
> Looks good to me. Let us propose this to Joe for review.
> 
Sent it. We probably still need to discuss about the remaining signatures:
1) "Debugged-by", 61
2) "Originally-by", 39
3) "Bisected-by", 20
4) "Diagnosed-by", 11
5) "Original-patch-by", 11
6) "Based-on-patch-by", 7
7) "Based-on-a-patch-by", 8
8) "Root-caused-by", 6
9) "Original-by", 6
10) "Based-on-patches-by", 5
11) "Based-on-work-by", 5

Should I send this list as a follow-up mail?

> Also, you can already start working on the related feature for
> providing fixes based on the edit distance.
> 
Okay.

Thanks
Aditya
> With both features, we can probably fix all non-standard signatures...
> 
> Lukas
> 
>> ---
>> changes in v2: replace commit specific example with brief evaluation
>>
>> changes in v3: provide rationale to users for every signature tag suggestion;
>> modify commit message describing arrival to conclusion in a structured way
>>
>>  scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 99 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index fdfd5ec09be6..575ff8efb0eb 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
>>         Cc:
>>  )};
>>
>> +our %standard_signature_fix = (
>> +       "Requested-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
>> +       },
>> +       "Co-authored-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Co-developed-by and Co-authored-by are synonyms",
>> +       },
>> +       "Analyzed-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Analysed-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Improvements-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Noticed-by:" => {
>> +               suggestion => "Reported-by:",
>> +               rationale => "Reported-by and Noticed-by are synonyms",
>> +       },
>> +       "Inspired-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
>> +       },
>> +       "Verified-by:" => {
>> +               suggestion => "Tested-by:",
>> +               rationale => "Tested-by and Verified-by are synonyms",
>> +       },
>> +       "Okay-ished-by:" => {
>> +               suggestion => "Acked-by:",
>> +               rationale => "Acked-by is the standard signature tag for recording your approval",
>> +       },
>> +       "Acked-for-MFD-by:" => {
>> +               suggestion => "Acked-by:",
>> +               rationale => "Acked-by is the standard signature tag for recording your approval",
>> +       },
>> +       "Reviewed-off-by:" => {
>> +               suggestion => "Reviewed-by:",
>> +               rationale => "Reviewed-by is the standard signature tag for recording your approval",
>> +       },
>> +       "Proposed-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
>> +       },
>> +       "Fixed-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Pointed-out-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
>> +       },
>> +       "Pointed-at-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
>> +       },
>> +       "Suggestions-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
>> +       },
>> +       "Generated-by:" => {
>> +               suggestion => "remove",
>> +               rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
>> +       },
>> +       "Celebrated-by:" => {
>> +               suggestion => "remove",
>> +               rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
>> +       },
>> +);
>> +
>>  our @typeListMisordered = (
>>         qr{char\s+(?:un)?signed},
>>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
>> @@ -2773,8 +2848,30 @@ sub process {
>>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>>
>>                         if ($sign_off !~ /$signature_tags/) {
>> -                               WARN("BAD_SIGN_OFF",
>> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
>> +                               my $suggested_signature = "";
>> +                               my $rationale = "";
>> +                               if (exists($standard_signature_fix{$sign_off})) {
>> +                                       $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
>> +                                       $rationale = $standard_signature_fix{$sign_off}{'rationale'};
>> +                               }
>> +                               if ($suggested_signature eq "") {
>> +                                       WARN("BAD_SIGN_OFF",
>> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
>> +                               }
>> +                               elsif ($suggested_signature eq "remove") {
>> +                                       if (WARN("BAD_SIGN_OFF",
>> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
>> +                                       $fix) {
>> +                                               fix_delete_line($fixlinenr, $rawline);
>> +                                       }
>> +                               }
>> +                               else {
>> +                                       if (WARN("BAD_SIGN_OFF",
>> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
>> +                                       $fix) {
>> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
>> +                                       }
>> +                               }
>>                         }
>>                         if (defined $space_before && $space_before ne "") {
>>                                 if (WARN("BAD_SIGN_OFF",
>> --
>> 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 v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Mon, Nov 23, 2020 at 4:16 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 23/11/20 6:39 pm, Lukas Bulwahn wrote:
> > On Mon, Nov 23, 2020 at 1:21 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
> >>
> >> Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
> >> styles.
> >>
> >> This warning occurs because of incorrect use of signature tags,
> >> e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
> >> signature tags, which may seem correct, but are not standard:
> >>
> >> 1) Requested-by (count: 48) => Suggested-by
> >> Rationale: In an open-source project, there are no 'requests', just
> >> 'suggestions' to convince a maintainer to accept your patch
> >>
> >> 2) Co-authored-by (count: 43) => Co-developed-by
> >> Rationale: Co-developed-by and Co-authored-by are synonyms
> >>
> >> 3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
> >> Rationale: Analyzing is a part of Software Development, so
> >> 'Co-developed-by' is perfectly fine, even if contributor did not create
> >> code
> >>
> >> 4) Improvements-by (count: 19) => Co-developed-by
> >>
> >> 5) Noticed-by (count: 11) => Reported-by
> >>
> >> 6) Inspired-by (count: 11) => Suggested-by
> >>
> >> 7) Verified-by (count: 8) => Tested-by
> >> Rationale: Used by a single user. On reading mailing list, it seems
> >> Tested-by might be a suitable alternative
> >>
> >> 8) Okay-ished-by (count: 8) => Acked-by
> >> Rationale: Used by a single user. On reading mailing list, it seems
> >> Acked-by must be suitable alternative
> >>
> >> 9) Acked-for-MFD-by (count: 6) => Acked-by
> >>
> >> 10) Reviewed-off-by (count: 5) => Reviewed-by
> >>
> >> 11) Proposed-by (count: 5) => Suggested-by
> >> Rationale: On observing the mailing list, this tag is always used for a
> >> maintainer. It seems that the changes might have been suggested by them
> >> and the tag is used as acknowledgement for the same
> >>
> >> 12) Fixed-by (count: 3) => Co-developed-by
> >> Rationale: Fixing bug is a part of Software Development, so
> >> 'Co-developed-by' is perfectly fine, even if contributor did not create
> >> code
> >>
> >> 13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
> >> Rationale: The tags are used for maintainers. It seems that the changes
> >> might have been suggested by them and the tag is used as acknowledgement
> >> for the same
> >> E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>
> >> 14) Suggestions-by (count: 3) => Suggested-by
> >>
> >> 15) Generated-by (count: 17) => remove the tag
> >> On observing the mailing list, this tag is always used for quoting the
> >> tool or script, which might have been used to generate the patch.
> >> E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
> >>
> >> 16) Celebrated-by (count: 3) => remove the tag
> >> This tag was used for only one commit. On observing mailing list, it seem
> >> like the celebration for a particular patch and changes.
> >>
> >> Provide a fix by:
> >> 1) replacing the non-standard signature with its standard equivalent
> >> 2) removing the signature if it is not required
> >>
> >> Also, improve warning messages correspondingly, providing users
> >> suggestions to either replace or remove the signature. Also provide
> >> suitable rationale to the user for the suggestion made.
> >>
> >> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> >
> >
> > Looks good to me. Let us propose this to Joe for review.
> >
> Sent it. We probably still need to discuss about the remaining signatures:
> 1) "Debugged-by", 61
> 2) "Originally-by", 39
> 3) "Bisected-by", 20
> 4) "Diagnosed-by", 11
> 5) "Original-patch-by", 11
> 6) "Based-on-patch-by", 7
> 7) "Based-on-a-patch-by", 8
> 8) "Root-caused-by", 6
> 9) "Original-by", 6
> 10) "Based-on-patches-by", 5
> 11) "Based-on-work-by", 5
>
> Should I send this list as a follow-up mail?
>

Let us give Joe time to review the patch first; then if accepted, we
can continue the discussion on those tags above; you can already
propose a solution you have in mind and think about where the
documentation, checkpatch etc. needs to be adjusted.

> > Also, you can already start working on the related feature for
> > providing fixes based on the edit distance.
> >
> Okay.
>
> Thanks
> Aditya
> > With both features, we can probably fix all non-standard signatures...
> >
> > Lukas
> >
> >> ---
> >> changes in v2: replace commit specific example with brief evaluation
> >>
> >> changes in v3: provide rationale to users for every signature tag suggestion;
> >> modify commit message describing arrival to conclusion in a structured way
> >>
> >>  scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 99 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >> index fdfd5ec09be6..575ff8efb0eb 100755
> >> --- a/scripts/checkpatch.pl
> >> +++ b/scripts/checkpatch.pl
> >> @@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
> >>         Cc:
> >>  )};
> >>
> >> +our %standard_signature_fix = (
> >> +       "Requested-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
> >> +       },
> >> +       "Co-authored-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Co-developed-by and Co-authored-by are synonyms",
> >> +       },
> >> +       "Analyzed-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Analysed-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Improvements-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Noticed-by:" => {
> >> +               suggestion => "Reported-by:",
> >> +               rationale => "Reported-by and Noticed-by are synonyms",
> >> +       },
> >> +       "Inspired-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> >> +       },
> >> +       "Verified-by:" => {
> >> +               suggestion => "Tested-by:",
> >> +               rationale => "Tested-by and Verified-by are synonyms",
> >> +       },
> >> +       "Okay-ished-by:" => {
> >> +               suggestion => "Acked-by:",
> >> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> >> +       },
> >> +       "Acked-for-MFD-by:" => {
> >> +               suggestion => "Acked-by:",
> >> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> >> +       },
> >> +       "Reviewed-off-by:" => {
> >> +               suggestion => "Reviewed-by:",
> >> +               rationale => "Reviewed-by is the standard signature tag for recording your approval",
> >> +       },
> >> +       "Proposed-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
> >> +       },
> >> +       "Fixed-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Pointed-out-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> >> +       },
> >> +       "Pointed-at-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> >> +       },
> >> +       "Suggestions-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> >> +       },
> >> +       "Generated-by:" => {
> >> +               suggestion => "remove",
> >> +               rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
> >> +       },
> >> +       "Celebrated-by:" => {
> >> +               suggestion => "remove",
> >> +               rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
> >> +       },
> >> +);
> >> +
> >>  our @typeListMisordered = (
> >>         qr{char\s+(?:un)?signed},
> >>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> >> @@ -2773,8 +2848,30 @@ sub process {
> >>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
> >>
> >>                         if ($sign_off !~ /$signature_tags/) {
> >> -                               WARN("BAD_SIGN_OFF",
> >> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> >> +                               my $suggested_signature = "";
> >> +                               my $rationale = "";
> >> +                               if (exists($standard_signature_fix{$sign_off})) {
> >> +                                       $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
> >> +                                       $rationale = $standard_signature_fix{$sign_off}{'rationale'};
> >> +                               }
> >> +                               if ($suggested_signature eq "") {
> >> +                                       WARN("BAD_SIGN_OFF",
> >> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
> >> +                               }
> >> +                               elsif ($suggested_signature eq "remove") {
> >> +                                       if (WARN("BAD_SIGN_OFF",
> >> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
> >> +                                       $fix) {
> >> +                                               fix_delete_line($fixlinenr, $rawline);
> >> +                                       }
> >> +                               }
> >> +                               else {
> >> +                                       if (WARN("BAD_SIGN_OFF",
> >> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
> >> +                                       $fix) {
> >> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> >> +                                       }
> >> +                               }
> >>                         }
> >>                         if (defined $space_before && $space_before ne "") {
> >>                                 if (WARN("BAD_SIGN_OFF",
> >> --
> >> 2.17.1
> >>
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees