git pre-commit hook running checkpatch.pl

git pre-commit hook running checkpatch.pl

[Ben Dooks]

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

I've written a git pre-commit[1] hook which stops any commits happening if
checkpatch.pl reports problems with the patch. Any comments?

[1] http://www.fluff.org/ben/linux/githooks/pre-commit

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: git pre-commit hook running checkpatch.pl

[Russell King - ARM Linux]

On Wed, Jan 20, 2010 at 02:02:02AM +0000, Ben Dooks wrote:
> I've written a git pre-commit[1] hook which stops any commits happening if
> checkpatch.pl reports problems with the patch. Any comments?

Very bad - checkpatch can be wrong at times.

Eg 1, asm/irq.h vs linux/irq.h
Eg 2, asm/memory.h vs linux/memory.h
Eg 3, lines longer than 80 characters for printk() strings are preferred
  so that they are grep-able.

checkpatch is at best a guide, not a set of hard rules.

Re: git pre-commit hook running checkpatch.pl

[Mark Brown]

On Wed, Jan 20, 2010 at 02:02:02AM +0000, Ben Dooks wrote:

> I've written a git pre-commit[1] hook which stops any commits happening if
> checkpatch.pl reports problems with the patch. Any comments?

> [1] http://www.fluff.org/ben/linux/githooks/pre-commit

Checkpatch does sometimes generate the occasional false positive which
can be annoying when they trigger.  It can also be a problem when doing
bugfixes IME - if an existing file has lots of checkpatch issues then a
fix which maintains the same coding style as the surrounding code can
trigger checkpatch issues which would if fixed make the code not fit in
with the rest of the file.  Obviously it'd be desirable to fix the file
but that's a bit unreasonable for people submitting bug fixes,
especially when pushing fixes to Linus and stable.

That said, monitoring for checkpatch issues is useful.

Re: git pre-commit hook running checkpatch.pl

[Uwe Kleine-König]

On Wed, Jan 20, 2010 at 10:01:55AM +0000, Mark Brown wrote:
> On Wed, Jan 20, 2010 at 02:02:02AM +0000, Ben Dooks wrote:
> 
> > I've written a git pre-commit[1] hook which stops any commits happening if
> > checkpatch.pl reports problems with the patch. Any comments?
> 
> > [1] http://www.fluff.org/ben/linux/githooks/pre-commit
> 
> Checkpatch does sometimes generate the occasional false positive which
> can be annoying when they trigger.  It can also be a problem when doing
> bugfixes IME - if an existing file has lots of checkpatch issues then a
> fix which maintains the same coding style as the surrounding code can
> trigger checkpatch issues which would if fixed make the code not fit in
> with the rest of the file.  Obviously it'd be desirable to fix the file
> but that's a bit unreasonable for people submitting bug fixes,
> especially when pushing fixes to Linus and stable.
> 
> That said, monitoring for checkpatch issues is useful.
Maybe only print the warnings instead of stopping a commit?  I'd not
like me stopping here not only for the reasons pointed out by Mark and
Russell, but also as I frequently do wip-commits that not necessarily
are checkpatch-clean.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |