Messages in this thread Patch in this message |  | | Subject | Re: [PATCH v5] checkpatch: add fix and improve warning msg for non-standard signature | From | Joe Perches <> | Date | Sat, 28 Nov 2020 11:12:59 -0800 |
| |
On Sun, 2020-11-29 at 00:05 +0530, Aditya Srivastava wrote: > Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature > styles.
Seems OK, but here are some last trivial notes:
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > +sub find_standard_signature { > + my ($sign_off) = @_; > + my @standard_signature_tags = ( > + 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:', > + 'reviewed-by:', 'reported-by:', 'suggested-by:'
I would change this to the normal signatures:
my @standard_signature_tags = ( 'Signed-off-by:', 'Co-developed-by:', 'Acked-by:', 'Tested-by:', 'Reviewed-by:', 'Reported-by:', 'Suggested-by:'
> + ); > + my $standard_signature; > + my $min_edit_distance = 20; # setting default value
20 seems arbitrary, maybe (~0 << 1) ?
> + my $edit_distance;
move this into the foreach (or maybe not use this at all)
> + foreach (@standard_signature_tags) {
foreach style in this code uses foreach my $<something> and not $_
foreach my $standard (@standard_signature_tags) {
> + $edit_distance = get_edit_distance($sign_off, $_);
So:
my $edit_distance = get_edit_distance($sign_off, $standard);
> + if ($edit_distance < $min_edit_distance) { > + $min_edit_distance = $edit_distance; > + $standard_signature = $_; > + } > + } > + if ($min_edit_distance <= 2) { > + return ucfirst($standard_signature);
return $standard;
Though maybe it's simpler to test in the loop if it's <= 2 as the lowercase and dash strip is done inside get_edit_distance so this seems rather simpler:
foreach my $standard (@standard_signature_tags) { return $standard if (get_edit_distance($sign_off, $standard) <= 2); }
return "";
> @@ -2773,8 +2839,17 @@ 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 = find_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. Perhaps '$suggested_signature'\n" . $herecurr) &&
Please use consistent '' or nothing around signatures:
"Non-standard signature: '$sign_off' - likely typo of '$suggested_signature'\n" . $herecurr) &&
> + $fix) { > + $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/; > + } > + } > } > if (defined $space_before && $space_before ne "") { > if (WARN("BAD_SIGN_OFF",
|  |