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

Lukas Bulwahn lukas.bulwahn at gmail.com
Sat Nov 21 09:52:41 UTC 2020


On Sat, Nov 21, 2020 at 5:58 AM Aditya Srivastava <yashsri421 at 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 at 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
>


More information about the Linux-kernel-mentees mailing list