Re: [PATCH v3] checkpatch: improve email parsing

From: Joe Perches
Date: Thu Nov 05 2020 - 19:26:29 EST


On Fri, 2020-11-06 at 02:46 +0530, Dwaipayan Ray wrote:
> > Can you send me a file with the BAD_SIGN_OFF messages generated
> > and if possible the git SHA-1s of the commits?
> Yes sure, am attaching the file for data tested on about 27k commits
> from v5.4.

Thanks.

> Excluded the duplicate signatures, tags and spacing ones for
> simplicity.
>
> >      24 linux-stable <stable@xxxxxxxxxxxxxxx>
> >      21 5.4+ <stable@xxxxxxxxxxxxxxx>
> >      14 All applicable <stable@xxxxxxxxxxxxxxx>
> >       6 3.10+ <stable@xxxxxxxxxxxxxxx>
> >       5 5.9+ <stable@xxxxxxxxxxxxxxx>
> >       5 5.3+ <stable@xxxxxxxxxxxxxxx>
>
> Do I also convert these then for the fix?

Yes.

Ideally removing any case insensitive name like linux-stable or stable
and also removing any leading < or trailing > from the email address.

And the stable email address should be forced to lower case only.

Any other name should be moved as a comment after the email address
then #.

> > > Improvements to parsing:
> > >
> > > - Detect and report unexpected content after email.
> > > - Quoted names are excluded from comment parsing.
> > > - Trailing dots or commas in email are removed during
> > >   formatting. Correspondingly a BAD_SIGN_OFF warning
> > >   is emitted.
> > > - Improperly quoted email like '"name <address>"' are now
> > >   warned about.
[]
> > > + my $new_comment = $comment;
> > > + $new_comment =~ s/^[ \(\[]+|[ \)\]]+$//g;
> >
> > Does the comment include any leading whitespace here?
> > I presumed not given the $comment !~ /^#/ test above.
> >
> I added it to discard empty spaces before or after the first
> brackets are unwrapped. Something like ( v5.7 ) would
> be better if translated to # v5.7. (The extra spaces would be purged).
> It just looks good :). Should I change it to keep it as it is?

No. Converting the comment style is very sensible and a good change.
I did not understand why the regex included a space. I do now.
I suggest changing the space to use \s in case there are tabs.