Re: [PATCH] checkpatch: do not test/warn of leading whitespace before signature tags

[Jeff Kirsher <jeffrey.t.kirsher@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2799 bytes --]

On Thu, 2011-08-18 at 00:59 -0700, Joe Perches wrote:
> On Thu, 2011-08-18 at 00:47 -0700, Jeff Kirsher wrote:
> > On Thu, 2011-08-18 at 00:26 -0700, Joe Perches wrote:
> > > On Wed, 2011-08-17 at 23:48 -0700, Jeff Kirsher wrote:
> > > > From: Bruce Allan <bruce.w.allan@intel.com>
> > > > 
> > > > Commit 2011247 introduced additional style checks for signature tags in
> > > > patches which is good.  Unfortunately, now whenever patches are checked
> > > > by piping the output of 'git show' or 'stg show' through checkpatch it
> > > > warns not to use whitespace before all signature tags since these (and the
> > > > rest of the patch description) are indented.  Remove this test/warning.
> > > 
> > > I think this is not a good idea.
> > > 
> > > checkpatch is meant for patches not git log output.
> > > indenting signatures can cause other problems later.
> > > 
> > > I think you can avoid this easily by using checkpatch
> > > option --ignore=BAD_SIGN_OFF when using git log output
> > > as input.
> > 
> > The problem I have with this is that the sign-off's are not bad, they
> > are by default indented by 'git show' or 'stg show' so checkpatch.pl
> > should handle the "default" formatting of git/stg and if there is
> > additional indenting not expected, then the sign-off's should be
> > considered bad.
> 
> I disagree.
> 
> checkpatch should handle the default input of patches
> as best it can.
> 
> I suppose checkpatch could have a different "--input=git"
> or some such to avoid certain things that git might produce
> that a patch would not.
> 
> Deleting useful checks for patches isn't a good idea.
> 
> > If this option is added, then if there were "real"
> > problems with the sign-off, it would not be displayed.
> 
> So what?
> 
> It would also be too late to do anything about
> it anyway as it would already be committed.

To add...

Specifically, most maintainers ask developers to run checkpatch.pl on
their patches before submitting them to the community.  As part of our
patch validation process, our validation team runs checkpatch.pl on
every patch I submit and we are now getting constant warnings about good
sign-off's with this recent change.  I personally end up reviewing all
the warnings to ensure that they are false warnings and not "real"
issues with the sign-off's.

> 
> > > You could also use: 
> > > git log --format="commit %H%nAuthor: %an <%ae>%nDate:   %aD%n%n%s%n%n%b"
> > > so that you get the current default --format=medium
> > > output without indenting the commit log body.
> > Even doing this does not resolve the "false" warnings" that
> > checkpatch.pl produces regarding the sign-off's.
> 
> I tried it.  It works for me.
> What about it doesn't work for you?
> 
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]