2019-05-20 12:48:32

by Michal Kalderon

[permalink] [raw]
Subject: [PATCH] checkpatch: add test for empty line after Fixes statement

Check that there is no empty line after a fixes statement

Reviewed-by: Leon Romanovsky <[email protected]>
Signed-off-by: Michal Kalderon <[email protected]>
---
scripts/checkpatch.pl | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a09333fd7cef..6cbc07364d4f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -62,6 +62,7 @@ my $conststructsfile = "$D/const_structs.checkpatch";
my $typedefsfile = "";
my $color = "auto";
my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
+my $fixes = 0;

sub help {
my ($exitcode) = @_;
@@ -2792,6 +2793,18 @@ sub process {
}
}

+# Check if Fixes statement to make sure next line is not blank
+ if ($fixes) {
+ if ($line =~ /^\s*$/) {
+ WARN("EMPTY_LINE_AFTER_FIXES", "No Empty line after Fixes statement\n" . $here);
+ }
+ $fixes = 0;
+ }
+
+ if ($in_commit_log && $line =~ /Fixes/) {
+ $fixes = 1;
+ }
+
# Check for added, moved or deleted files
if (!$reported_maintainer_file && !$in_commit_log &&
($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||
--
2.14.5



2019-05-20 17:58:50

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: add test for empty line after Fixes statement

On Mon, 2019-05-20 at 15:42 +0300, Michal Kalderon wrote:
> Check that there is no empty line after a fixes statement

why?



2019-05-20 18:00:20

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: add test for empty line after Fixes statement

On Mon, May 20, 2019 at 05:56:36AM -0700, Joe Perches wrote:
> On Mon, 2019-05-20 at 15:42 +0300, Michal Kalderon wrote:
> > Check that there is no empty line after a fixes statement
>
> why?

It is common mistake for Gerrit users, they are removing
their ChangeID crap with some wrong sed command which leaves
empty line.

You can argue that this should be fixed on the client side and I agree,
nut because the checkpatch check is so easy, it is worth to add it and
save reviewers time.

Thanks

>
>

2019-05-20 18:00:28

by Michal Kalderon

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes statement

> From: Joe Perches <[email protected]>
> Sent: Monday, May 20, 2019 3:57 PM
> Subject: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes
> statement
>
> External Email
>
> ----------------------------------------------------------------------
> On Mon, 2019-05-20 at 15:42 +0300, Michal Kalderon wrote:
> > Check that there is no empty line after a fixes statement
>
> why?
>
This comment is given a lot on the netdev and rdma mailing lists when patches are submitted with
an empty line between Fixes: tag and SOB tags. Since "Fixes:" is just another tag and should be kept
together with the other ones.
thanks,
Michal

2019-05-20 18:03:19

by Joe Perches

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes statement

On Mon, 2019-05-20 at 13:16 +0000, Michal Kalderon wrote:
> > From: Joe Perches <[email protected]>
> > Sent: Monday, May 20, 2019 3:57 PM
> > Subject: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes
> > statement
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Mon, 2019-05-20 at 15:42 +0300, Michal Kalderon wrote:
> > > Check that there is no empty line after a fixes statement
> >
> > why?
> >
> This comment is given a lot on the netdev and rdma mailing lists when patches are submitted with
> an empty line between Fixes: tag and SOB tags. Since "Fixes:" is just another tag and should be kept
> together with the other ones.

So test that all signature blocks and Fixes do not have
blank lines between them instead of just the "Fixes:" line.

And if there is some specific ordering required, perhaps a
test for that ordering should be added as well.


2019-05-20 18:52:02

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes statement

On Mon, May 20, 2019 at 06:34:49AM -0700, Joe Perches wrote:
> On Mon, 2019-05-20 at 13:16 +0000, Michal Kalderon wrote:
> > > From: Joe Perches <[email protected]>
> > > Sent: Monday, May 20, 2019 3:57 PM
> > > Subject: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes
> > > statement
> > >
> > > External Email
> > >
> > > ----------------------------------------------------------------------
> > > On Mon, 2019-05-20 at 15:42 +0300, Michal Kalderon wrote:
> > > > Check that there is no empty line after a fixes statement
> > >
> > > why?
> > >
> > This comment is given a lot on the netdev and rdma mailing lists when patches are submitted with
> > an empty line between Fixes: tag and SOB tags. Since "Fixes:" is just another tag and should be kept
> > together with the other ones.
>
> So test that all signature blocks and Fixes do not have
> blank lines between them instead of just the "Fixes:" line.
>
> And if there is some specific ordering required, perhaps a
> test for that ordering should be added as well.

I'm aware of only one request - Fixes above SOB.

Thanks

>

2019-05-21 00:10:26

by Joe Perches

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes statement

On Mon, 2019-05-20 at 16:52 +0300, Leon Romanovsky wrote:
> On Mon, May 20, 2019 at 06:34:49AM -0700, Joe Perches wrote:
> > On Mon, 2019-05-20 at 13:16 +0000, Michal Kalderon wrote:
> > > > From: Joe Perches <[email protected]>
> > > > Sent: Monday, May 20, 2019 3:57 PM
> > > > Subject: [EXT] Re: [PATCH] checkpatch: add test for empty line after Fixes
> > > > statement
> > > >
> > > > External Email
> > > >
> > > > ----------------------------------------------------------------------
> > > > On Mon, 2019-05-20 at 15:42 +0300, Michal Kalderon wrote:
> > > > > Check that there is no empty line after a fixes statement
> > > >
> > > > why?
> > > >
> > > This comment is given a lot on the netdev and rdma mailing lists when patches are submitted with
> > > an empty line between Fixes: tag and SOB tags. Since "Fixes:" is just another tag and should be kept
> > > together with the other ones.
> >
> > So test that all signature blocks and Fixes do not have
> > blank lines between them instead of just the "Fixes:" line.
> >
> > And if there is some specific ordering required, perhaps a
> > test for that ordering should be added as well.
>
> I'm aware of only one request - Fixes above SOB.

Well, nack for the suggested patch.

If there are signature blocks, then there should not be blank lines
between entries and there should be a blank line before the
signature block.

The current documentation in Documentation/process/submitting-patches.rst
doesn't state anything about blank lines above or below Fixes: lines.