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

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

[Michal Kalderon]

Check that there is no empty line after a fixes statement

Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Michal Kalderon <michal.kalderon@marvell.com>
---
 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

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

[Joe Perches]

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

why?

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

[Leon Romanovsky]

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

>
>

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

[Michal Kalderon]

> From: Joe Perches <joe@perches.com>
> 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

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

[Joe Perches]

On Mon, 2019-05-20 at 13:16 +0000, Michal Kalderon wrote:
> > From: Joe Perches <joe@perches.com>
> > 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.

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

[Leon Romanovsky]

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 <joe@perches.com>
> > > 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

>

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

[Joe Perches]

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 <joe@perches.com>
> > > > 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.