Am Donnerstag, den 21.04.2011, 17:46 +0200 schrieb Detlev Zundel:
Thanks for the input. Current base for discussion:
Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results.
Warnings that clearly only make sense in the Linux kernel can be ignored. This includes "Use #include <linux/$file> instead of <asm/$file>" for example.
If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch.
For minor modifications (e.g. changed arguments of a function call), adhere to the present codingstyle of the module. Relating checkpatch warnings can be ignored in this case. A respective note in the commit or cover letter why they are ignored is desired.
Anyone unhappy with that? Will this help newcomers?
Sounds sensible to me. The extra paragraph above is to circumvent intermixing codingstyle inside a file. For example in common/main.c, there are "function (arg, arg)" all around. Not checkpatch compliant (and ugly, IMHO). Now when I modify one of this calls, e.g add an argument, I'd have to change this. Which leads to a mixture of styles in the file, which is even worse than than adhering to the "broken" style.
In general, I'd suggest a - maybe informal - policy that while codingstyle is important, there are more relevant aspects like elegant code, sensible interfaces, proper use of static and const and so on. I'm sure any developer is willing to debate about these issues, but most volunteers (esp. company or freelance coders) will resign after the 3rd round of whitespace ping-pong...
Not sure how to phrase that without contradicting the basic idea, maybe with an additional paragraph like: "New modules have to be as checkpatch compliant as possible. Violations have to be justified in the commit or cover letter."