Hi Andy/Joe,
I got a warning today for my cover-letter, and it looked like a false
positive. Please have a look, based of v4.2-rc2.
-----------------------
0000-cover-letter.patch
-----------------------
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#31:
arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
--
viresh
On Thu, 2015-07-16 at 16:25 +0530, Viresh Kumar wrote:
> Hi Andy/Joe,
>
> I got a warning today for my cover-letter, and it looked like a false
> positive. Please have a look, based of v4.2-rc2.
> -----------------------
> 0000-cover-letter.patch
> -----------------------
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #31:
> arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
There are a lot of other false positives for this test.
Maybe it's not worth checking for this condition and the
test should be removed as "fixing" it may not be feasible.
On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > #31:
> > arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
I guess those are in the limbo land between the end of message and
beginning of the patch itself. Perhaps the test should at least stop at
the end of header marker, at the '---'.
-apw
On Thu, 2015-07-16 at 16:43 +0100, Andy Whitcroft wrote:
> On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > > #31:
> > > arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
>
> I guess those are in the limbo land between the end of message and
> beginning of the patch itself. Perhaps the test should at least stop at
> the end of header marker, at the '---'.
>
> -apw
Maybe, but the test already stops at signatures like
Signed-off-by: that should always be above the ---.
This might help, but there are _many_ false positives.
The other thing that might help is for people to take
the warnings the script produces less seriously.
Maybe convert:
ERROR -> defect
WARNING -> unstylish
CHECK -> nitpick
or some such
---
scripts/checkpatch.pl | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d5ce29a..5e7afa7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2213,6 +2213,11 @@ sub process {
$in_commit_log = 0;
}
+# Check for patch separator
+ if ($line =~ /^---$/) {
+ $in_commit_log = 0;
+ }
+
# Check if MAINTAINERS is being updated. If so, there's probably no need to
# emit the "does MAINTAINERS need updating?" message on file add/move/delete
if ($line =~ /^\s*MAINTAINERS\s*\|/) {
On Thu, Jul 16, 2015 at 08:58:56AM -0700, Joe Perches wrote:
> On Thu, 2015-07-16 at 16:43 +0100, Andy Whitcroft wrote:
> > On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > > > #31:
> > > > arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
> >
> > I guess those are in the limbo land between the end of message and
> > beginning of the patch itself. Perhaps the test should at least stop at
> > the end of header marker, at the '---'.
> >
> > -apw
>
> Maybe, but the test already stops at signatures like
> Signed-off-by: that should always be above the ---.
>
> This might help, but there are _many_ false positives.
>
> The other thing that might help is for people to take
> the warnings the script produces less seriously.
>
> Maybe convert:
>
> ERROR -> defect
> WARNING -> unstylish
> CHECK -> nitpick
Heh, that has long been the main issue, please please believe your brain
not checkpatch. But yes some less inflamitory words might, just might,
reduce the noise.
-apw