[coreboot] What is the best way to treat warnings reported by checkpatch.pl

Julius Werner jwerner at chromium.org
Wed Jul 27 02:04:31 CEST 2016


Does git commit --no-verify (or -n for short) allow you to commit?

I think we should try to do a little of option 1 within reason, not by
forking the script but rather by disabling more check steps that don't
match the coreboot code style (by having our wrapper pass --ignore XXX
flags to checkpatch) and possibly upstreaming checkpatch.pl patches
with new features we need to the Linux kernel (to make the detection
more accurate or add a more specific --ignore flag we can turn on). In
the Chromium fork of coreboot we're already using a bunch of those
flags that we should probably use in upstream coreboot as well:

--ignore C99_COMMENTS
--ignore GLOBAL_INITIALISERS
--ignore INITIALISED_STATIC
--ignore LINE_SPACING
--ignore NEW_TYPEDEFS
--ignore PREFER_ALIGNED
--ignore PREFER_PACKED
--ignore PREFER_PRINTF
--ignore SPLIT_STRING

However, in my experience there will always be edge cases checkpatch
doesn't get right, no matter how well you tweak it. It should always
be up to the discretion of the committer/reviewer which warnings they
act upon, so we need to retain a mechanism (like --no-verify) to work
around it if necessary. Maybe we should update the pre-commit hook to
print an explanation to this effect. (Ideally, I think it would be
awesome if Gerrit could separately run checkpatch and make it explicit
in the interface if someone circumvented a warning this way so it can
be discussed, but that's probably not and easy thing to add.)



More information about the coreboot mailing list