* [PATCH v4] checkpatch: improve email parsing
@ 2020-11-06 21:45 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-06 21:45 UTC (permalink / raw)
To: joe
Cc: linux-kernel-mentees, dwaipayanray1, linux-kernel, lukas.bulwahn,
yashsri421
checkpatch doesn't report warnings for many common mistakes
in emails. Some of which are trailing commas and incorrect
use of email comments.
At the same time several false positives are reported due to
incorrect handling of mail comments. The most common of which
is due to the pattern:
<stable@vger.kernel.org> # X.X
Improve email parsing in checkpatch.
Some general email rules are defined:
- Multiple name comments should not be allowed.
- Comments inside address should not be allowed.
- In general comments should be enclosed within parentheses.
Relaxation is given to comments beginning with #.
- Stable addresses should not begin with a name.
- Comments in stable addresses should begin only
with a #.
Improvements to parsing:
- Detect and report unexpected content after email.
- Quoted names are excluded from comment parsing.
- Trailing dots, commas or quotes in email are removed during
formatting. Correspondingly a BAD_SIGN_OFF warning
is emitted.
- Improperly quoted email like '"name <address>"' are now
warned about.
In addition, added fixes for all the possible rules.
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++--------
1 file changed, 80 insertions(+), 17 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..d866c4321182 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1152,6 +1152,7 @@ sub parse_email {
my ($formatted_email) = @_;
my $name = "";
+ my $quoted = "";
my $name_comment = "";
my $address = "";
my $comment = "";
@@ -1183,14 +1184,20 @@ sub parse_email {
}
}
- $comment = trim($comment);
- $name = trim($name);
- $name =~ s/^\"|\"$//g;
- if ($name =~ s/(\s*\([^\)]+\))\s*//) {
- $name_comment = trim($1);
+ # Extract comments from names excluding quoted parts
+ # "John A. (Kennedy)" - Do not extract
+ if ($name =~ s/\"(.+)\"//) {
+ $quoted = $1;
+ }
+ while ($name =~ s/\s*($balanced_parens)\s*/ /) {
+ $name_comment .= trim($1);
}
+ $name =~ s/^[ \"]+|[ \"]+$//g;
+ $name = trim("$quoted $name");
+
$address = trim($address);
$address =~ s/^\<|\>$//g;
+ $comment = trim($comment);
if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
@@ -1205,17 +1212,20 @@ sub format_email {
my $formatted_email;
- $name_comment = trim($name_comment);
- $comment = trim($comment);
- $name = trim($name);
- $name =~ s/^\"|\"$//g;
+ $name =~ s/^[ \"]+|[ \"]+$//g;
$address = trim($address);
+ $address =~ s/(?:\.|\,|\")+$//; ##trailing commas, dots or quotes
if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
$name = "\"$name\"";
}
+ $name_comment = trim($name_comment);
+ $name_comment = " $name_comment" if length($name_comment) > 0;
+ $comment = trim($comment);
+ $comment = " $comment" if length($comment) > 0;
+
if ("$name" eq "") {
$formatted_email = "$address";
} else {
@@ -1233,15 +1243,11 @@ sub reformat_email {
}
sub same_email_addresses {
- my ($email1, $email2, $match_comment) = @_;
+ my ($email1, $email2) = @_;
my ($email1_name, $name1_comment, $email1_address, $comment1) = parse_email($email1);
my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2);
- if ($match_comment != 1) {
- return $email1_name eq $email2_name &&
- $email1_address eq $email2_address;
- }
return $email1_name eq $email2_name &&
$email1_address eq $email2_address &&
$name1_comment eq $name2_comment &&
@@ -2704,7 +2710,7 @@ sub process {
$signoff++;
$in_commit_log = 0;
if ($author ne '' && $authorsignoff != 1) {
- if (same_email_addresses($1, $author, 1)) {
+ if (same_email_addresses($1, $author)) {
$authorsignoff = 1;
} else {
my $ctx = $1;
@@ -2800,9 +2806,66 @@ sub process {
$dequoted =~ s/" </ </;
# Don't force email to have quotes
# Allow just an angle bracketed address
- if (!same_email_addresses($email, $suggested_email, 0)) {
+ if (!same_email_addresses($email, $suggested_email)) {
+ if (WARN("BAD_SIGN_OFF",
+ "email address '$email' might be better as '$suggested_email'\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\Q$email\E/$suggested_email/;
+ }
+ }
+
+ # Address part shouldn't have comments
+ my $stripped_address = $email_address;
+ $stripped_address =~ s/\([^\(\)]*\)//g;
+ if ($email_address ne $stripped_address) {
+ if (WARN("BAD_SIGN_OFF",
+ "address part of email should not have comments: '$email_address'\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\Q$email_address\E/$stripped_address/;
+ }
+ }
+
+ # Only one name comment should be allowed
+ my $comment_count = () = $name_comment =~ /\([^\)]+\)/g;
+ if ($comment_count > 1) {
WARN("BAD_SIGN_OFF",
- "email address '$email' might be better as '$suggested_email'\n" . $herecurr);
+ "Use a single name comment in email: '$email'\n" . $herecurr);
+ }
+
+
+ # stable@vger.kernel.org or stable@kernel.org shouldn't
+ # have an email name. In addition commments should strictly
+ # begin with a #
+ if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
+ if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
+ ($email_name ne ""))) {
+ my $cur_name = $email_name;
+ my $new_comment = $comment;
+
+ $cur_name =~ s/[a-zA-Z\s\-\"]+//g;
+ $new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
+ $new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
+ $new_comment = " # $new_comment" if length($new_comment) > 0;
+ my $new_email = "$email_address$new_comment";
+
+ if (WARN("BAD_SIGN_OFF",
+ "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
+ }
+ }
+ } else {
+ if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
+ if (WARN("BAD_SIGN_OFF",
+ "Unexpected content after email: '$email'\n" . $herecurr) &&
+ $fix) {
+ my $new_comment = $comment;
+ $new_comment =~ s/^(?:\/\*|\.|\,)//g;
+ $new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
+ $new_comment = " ($new_comment)" if length($new_comment) > 0;
+ $fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
+ }
+ }
}
}
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing
@ 2020-11-06 21:45 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-06 21:45 UTC (permalink / raw)
To: joe; +Cc: dwaipayanray1, yashsri421, linux-kernel-mentees, linux-kernel
checkpatch doesn't report warnings for many common mistakes
in emails. Some of which are trailing commas and incorrect
use of email comments.
At the same time several false positives are reported due to
incorrect handling of mail comments. The most common of which
is due to the pattern:
<stable@vger.kernel.org> # X.X
Improve email parsing in checkpatch.
Some general email rules are defined:
- Multiple name comments should not be allowed.
- Comments inside address should not be allowed.
- In general comments should be enclosed within parentheses.
Relaxation is given to comments beginning with #.
- Stable addresses should not begin with a name.
- Comments in stable addresses should begin only
with a #.
Improvements to parsing:
- Detect and report unexpected content after email.
- Quoted names are excluded from comment parsing.
- Trailing dots, commas or quotes in email are removed during
formatting. Correspondingly a BAD_SIGN_OFF warning
is emitted.
- Improperly quoted email like '"name <address>"' are now
warned about.
In addition, added fixes for all the possible rules.
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++--------
1 file changed, 80 insertions(+), 17 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..d866c4321182 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1152,6 +1152,7 @@ sub parse_email {
my ($formatted_email) = @_;
my $name = "";
+ my $quoted = "";
my $name_comment = "";
my $address = "";
my $comment = "";
@@ -1183,14 +1184,20 @@ sub parse_email {
}
}
- $comment = trim($comment);
- $name = trim($name);
- $name =~ s/^\"|\"$//g;
- if ($name =~ s/(\s*\([^\)]+\))\s*//) {
- $name_comment = trim($1);
+ # Extract comments from names excluding quoted parts
+ # "John A. (Kennedy)" - Do not extract
+ if ($name =~ s/\"(.+)\"//) {
+ $quoted = $1;
+ }
+ while ($name =~ s/\s*($balanced_parens)\s*/ /) {
+ $name_comment .= trim($1);
}
+ $name =~ s/^[ \"]+|[ \"]+$//g;
+ $name = trim("$quoted $name");
+
$address = trim($address);
$address =~ s/^\<|\>$//g;
+ $comment = trim($comment);
if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
@@ -1205,17 +1212,20 @@ sub format_email {
my $formatted_email;
- $name_comment = trim($name_comment);
- $comment = trim($comment);
- $name = trim($name);
- $name =~ s/^\"|\"$//g;
+ $name =~ s/^[ \"]+|[ \"]+$//g;
$address = trim($address);
+ $address =~ s/(?:\.|\,|\")+$//; ##trailing commas, dots or quotes
if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
$name = "\"$name\"";
}
+ $name_comment = trim($name_comment);
+ $name_comment = " $name_comment" if length($name_comment) > 0;
+ $comment = trim($comment);
+ $comment = " $comment" if length($comment) > 0;
+
if ("$name" eq "") {
$formatted_email = "$address";
} else {
@@ -1233,15 +1243,11 @@ sub reformat_email {
}
sub same_email_addresses {
- my ($email1, $email2, $match_comment) = @_;
+ my ($email1, $email2) = @_;
my ($email1_name, $name1_comment, $email1_address, $comment1) = parse_email($email1);
my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2);
- if ($match_comment != 1) {
- return $email1_name eq $email2_name &&
- $email1_address eq $email2_address;
- }
return $email1_name eq $email2_name &&
$email1_address eq $email2_address &&
$name1_comment eq $name2_comment &&
@@ -2704,7 +2710,7 @@ sub process {
$signoff++;
$in_commit_log = 0;
if ($author ne '' && $authorsignoff != 1) {
- if (same_email_addresses($1, $author, 1)) {
+ if (same_email_addresses($1, $author)) {
$authorsignoff = 1;
} else {
my $ctx = $1;
@@ -2800,9 +2806,66 @@ sub process {
$dequoted =~ s/" </ </;
# Don't force email to have quotes
# Allow just an angle bracketed address
- if (!same_email_addresses($email, $suggested_email, 0)) {
+ if (!same_email_addresses($email, $suggested_email)) {
+ if (WARN("BAD_SIGN_OFF",
+ "email address '$email' might be better as '$suggested_email'\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\Q$email\E/$suggested_email/;
+ }
+ }
+
+ # Address part shouldn't have comments
+ my $stripped_address = $email_address;
+ $stripped_address =~ s/\([^\(\)]*\)//g;
+ if ($email_address ne $stripped_address) {
+ if (WARN("BAD_SIGN_OFF",
+ "address part of email should not have comments: '$email_address'\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\Q$email_address\E/$stripped_address/;
+ }
+ }
+
+ # Only one name comment should be allowed
+ my $comment_count = () = $name_comment =~ /\([^\)]+\)/g;
+ if ($comment_count > 1) {
WARN("BAD_SIGN_OFF",
- "email address '$email' might be better as '$suggested_email'\n" . $herecurr);
+ "Use a single name comment in email: '$email'\n" . $herecurr);
+ }
+
+
+ # stable@vger.kernel.org or stable@kernel.org shouldn't
+ # have an email name. In addition commments should strictly
+ # begin with a #
+ if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
+ if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
+ ($email_name ne ""))) {
+ my $cur_name = $email_name;
+ my $new_comment = $comment;
+
+ $cur_name =~ s/[a-zA-Z\s\-\"]+//g;
+ $new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
+ $new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
+ $new_comment = " # $new_comment" if length($new_comment) > 0;
+ my $new_email = "$email_address$new_comment";
+
+ if (WARN("BAD_SIGN_OFF",
+ "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
+ }
+ }
+ } else {
+ if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
+ if (WARN("BAD_SIGN_OFF",
+ "Unexpected content after email: '$email'\n" . $herecurr) &&
+ $fix) {
+ my $new_comment = $comment;
+ $new_comment =~ s/^(?:\/\*|\.|\,)//g;
+ $new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
+ $new_comment = " ($new_comment)" if length($new_comment) > 0;
+ $fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
+ }
+ }
}
}
--
2.27.0
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] checkpatch: improve email parsing
2020-11-06 21:45 ` [Linux-kernel-mentees] " Dwaipayan Ray
@ 2020-11-06 22:03 ` Joe Perches
-1 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-11-06 22:03 UTC (permalink / raw)
To: Dwaipayan Ray
Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn, yashsri421
On Sat, 2020-11-07 at 03:15 +0530, Dwaipayan Ray wrote:
> checkpatch doesn't report warnings for many common mistakes
> in emails. Some of which are trailing commas and incorrect
> use of email comments.
Assuming it all works, this looks good. I haven't tested it.
How did you test the $fix bits?
Trivial notes:
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> + # stable@vger.kernel.org or stable@kernel.org shouldn't
> + # have an email name. In addition commments should strictly
> + # begin with a #
> + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
Likely better to test with a case insensitive match so
STABLE@vger.kernel.org and such are still warned.
if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
> + if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> + ($email_name ne ""))) {
|| $sign_off !~ /^cc:/i ?
> + my $cur_name = $email_name;
> + my $new_comment = $comment;
> +
> + $cur_name =~ s/[a-zA-Z\s\-\"]+//g;
> + $new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
> + $new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
> + $new_comment = " # $new_comment" if length($new_comment) > 0;
> + my $new_email = "$email_address$new_comment";
> +
> + if (WARN("BAD_SIGN_OFF",
> + "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&
These message lines should be aligned to the next open parenthesis
(7 tabs, 1 space)
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
> + }
As a cc to stable isn't a sign-off, perhaps this should be a different
"BAD_<FOO>" like "BAD_STABLE_ADDRESS_STYLE" or such.
> + }
> + } else {
elsif ?
> + if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
> + if (WARN("BAD_SIGN_OFF",
> + "Unexpected content after email: '$email'\n" . $herecurr) &&
7 tabs, 1 space
> + $fix) {
> + my $new_comment = $comment;
> + $new_comment =~ s/^(?:\/\*|\.|\,)//g;
> + $new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
> + $new_comment = " ($new_comment)" if length($new_comment) > 0;
> + $fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
> + }
> + }
> }
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing
@ 2020-11-06 22:03 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-11-06 22:03 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel, yashsri421
On Sat, 2020-11-07 at 03:15 +0530, Dwaipayan Ray wrote:
> checkpatch doesn't report warnings for many common mistakes
> in emails. Some of which are trailing commas and incorrect
> use of email comments.
Assuming it all works, this looks good. I haven't tested it.
How did you test the $fix bits?
Trivial notes:
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> + # stable@vger.kernel.org or stable@kernel.org shouldn't
> + # have an email name. In addition commments should strictly
> + # begin with a #
> + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
Likely better to test with a case insensitive match so
STABLE@vger.kernel.org and such are still warned.
if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
> + if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> + ($email_name ne ""))) {
|| $sign_off !~ /^cc:/i ?
> + my $cur_name = $email_name;
> + my $new_comment = $comment;
> +
> + $cur_name =~ s/[a-zA-Z\s\-\"]+//g;
> + $new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
> + $new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
> + $new_comment = " # $new_comment" if length($new_comment) > 0;
> + my $new_email = "$email_address$new_comment";
> +
> + if (WARN("BAD_SIGN_OFF",
> + "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&
These message lines should be aligned to the next open parenthesis
(7 tabs, 1 space)
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
> + }
As a cc to stable isn't a sign-off, perhaps this should be a different
"BAD_<FOO>" like "BAD_STABLE_ADDRESS_STYLE" or such.
> + }
> + } else {
elsif ?
> + if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
> + if (WARN("BAD_SIGN_OFF",
> + "Unexpected content after email: '$email'\n" . $herecurr) &&
7 tabs, 1 space
> + $fix) {
> + my $new_comment = $comment;
> + $new_comment =~ s/^(?:\/\*|\.|\,)//g;
> + $new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
> + $new_comment = " ($new_comment)" if length($new_comment) > 0;
> + $fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
> + }
> + }
> }
> }
>
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] checkpatch: improve email parsing
2020-11-06 22:03 ` [Linux-kernel-mentees] " Joe Perches
@ 2020-11-07 4:41 ` Dwaipayan Ray
-1 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-07 4:41 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn, Aditya Srivastava
On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-11-07 at 03:15 +0530, Dwaipayan Ray wrote:
> > checkpatch doesn't report warnings for many common mistakes
> > in emails. Some of which are trailing commas and incorrect
> > use of email comments.
>
> Assuming it all works, this looks good. I haven't tested it.
>
> How did you test the $fix bits?
>
Hi,
I actually dumped about 17k unique emails from git log, put it in one of
my previous patches, and ran checkpatch with --fix on it.
I checked the diff and most of the cases looked pretty good to me.
I could send the diff output if you like?
> Trivial notes:
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > + # stable@vger.kernel.org or stable@kernel.org shouldn't
> > + # have an email name. In addition commments should strictly
> > + # begin with a #
> > + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
>
> Likely better to test with a case insensitive match so
> STABLE@vger.kernel.org and such are still warned.
Sure, I will do that.
>
> if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
>
> > + if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > + ($email_name ne ""))) {
>
> || $sign_off !~ /^cc:/i ?
I actually had a doubt about that one. Only the stable address with
Cc: should be checked right? Or something else?
What about those stable addresses with tags other than Cc: ? Should
a change be suggested?
Thanks,
Dwaipayan.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing
@ 2020-11-07 4:41 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-07 4:41 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, Aditya Srivastava
On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-11-07 at 03:15 +0530, Dwaipayan Ray wrote:
> > checkpatch doesn't report warnings for many common mistakes
> > in emails. Some of which are trailing commas and incorrect
> > use of email comments.
>
> Assuming it all works, this looks good. I haven't tested it.
>
> How did you test the $fix bits?
>
Hi,
I actually dumped about 17k unique emails from git log, put it in one of
my previous patches, and ran checkpatch with --fix on it.
I checked the diff and most of the cases looked pretty good to me.
I could send the diff output if you like?
> Trivial notes:
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > + # stable@vger.kernel.org or stable@kernel.org shouldn't
> > + # have an email name. In addition commments should strictly
> > + # begin with a #
> > + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
>
> Likely better to test with a case insensitive match so
> STABLE@vger.kernel.org and such are still warned.
Sure, I will do that.
>
> if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
>
> > + if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > + ($email_name ne ""))) {
>
> || $sign_off !~ /^cc:/i ?
I actually had a doubt about that one. Only the stable address with
Cc: should be checked right? Or something else?
What about those stable addresses with tags other than Cc: ? Should
a change be suggested?
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] checkpatch: improve email parsing
2020-11-07 4:41 ` [Linux-kernel-mentees] " Dwaipayan Ray
@ 2020-11-07 5:09 ` Joe Perches
-1 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-11-07 5:09 UTC (permalink / raw)
To: Dwaipayan Ray
Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn, Aditya Srivastava
On Sat, 2020-11-07 at 10:11 +0530, Dwaipayan Ray wrote:
> On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
> >
> > On Sat, 2020-11-07 at 03:15 +0530, Dwaipayan Ray wrote:
> > > checkpatch doesn't report warnings for many common mistakes
> > > in emails. Some of which are trailing commas and incorrect
> > > use of email comments.
> >
> > Assuming it all works, this looks good. I haven't tested it.
> >
> > How did you test the $fix bits?
> >
> Hi,
> I actually dumped about 17k unique emails from git log, put it in one of
> my previous patches, and ran checkpatch with --fix on it.
> I checked the diff and most of the cases looked pretty good to me.
> I could send the diff output if you like?
Please. Likely just to me as I imagine it's not interesting to most.
> > Trivial notes:
> >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > + # stable@vger.kernel.org or stable@kernel.org shouldn't
> > > + # have an email name. In addition commments should strictly
> > > + # begin with a #
> > > + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
> >
> > Likely better to test with a case insensitive match so
> > STABLE@vger.kernel.org and such are still warned.
>
> Sure, I will do that.
> >
> > if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
> >
> > > + if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > > + ($email_name ne ""))) {
> >
> > > > $sign_off !~ /^cc:/i ?
>
> I actually had a doubt about that one. Only the stable address with
> Cc: should be checked right? Or something else?
yes.
> What about those stable addresses with tags other than Cc: ? Should
> a change be suggested?
Ideally yes, but there were very few of those in the git commit
history so it's probably not a big deal one way or another.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing
@ 2020-11-07 5:09 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-11-07 5:09 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel, Aditya Srivastava
On Sat, 2020-11-07 at 10:11 +0530, Dwaipayan Ray wrote:
> On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
> >
> > On Sat, 2020-11-07 at 03:15 +0530, Dwaipayan Ray wrote:
> > > checkpatch doesn't report warnings for many common mistakes
> > > in emails. Some of which are trailing commas and incorrect
> > > use of email comments.
> >
> > Assuming it all works, this looks good. I haven't tested it.
> >
> > How did you test the $fix bits?
> >
> Hi,
> I actually dumped about 17k unique emails from git log, put it in one of
> my previous patches, and ran checkpatch with --fix on it.
> I checked the diff and most of the cases looked pretty good to me.
> I could send the diff output if you like?
Please. Likely just to me as I imagine it's not interesting to most.
> > Trivial notes:
> >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > + # stable@vger.kernel.org or stable@kernel.org shouldn't
> > > + # have an email name. In addition commments should strictly
> > > + # begin with a #
> > > + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
> >
> > Likely better to test with a case insensitive match so
> > STABLE@vger.kernel.org and such are still warned.
>
> Sure, I will do that.
> >
> > if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
> >
> > > + if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > > + ($email_name ne ""))) {
> >
> > > > $sign_off !~ /^cc:/i ?
>
> I actually had a doubt about that one. Only the stable address with
> Cc: should be checked right? Or something else?
yes.
> What about those stable addresses with tags other than Cc: ? Should
> a change be suggested?
Ideally yes, but there were very few of those in the git commit
history so it's probably not a big deal one way or another.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-07 5:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-06 21:45 [PATCH v4] checkpatch: improve email parsing Dwaipayan Ray
2020-11-06 21:45 ` [Linux-kernel-mentees] " Dwaipayan Ray
2020-11-06 22:03 ` Joe Perches
2020-11-06 22:03 ` [Linux-kernel-mentees] " Joe Perches
2020-11-07 4:41 ` Dwaipayan Ray
2020-11-07 4:41 ` [Linux-kernel-mentees] " Dwaipayan Ray
2020-11-07 5:09 ` Joe Perches
2020-11-07 5:09 ` [Linux-kernel-mentees] " Joe Perches
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.