Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups- formatting only

From: Ingo Molnar
Date: Tue Mar 25 2008 - 10:03:53 EST



* Jörn Engel <joern@xxxxxxxxx> wrote:

> > So to turn around the argument: could you give me any reason why
> > differing coding style between subsystems, _often in blatant
> > violation of Documentation/CodingStyle_, is somehow "good" for Linux
> > in the long run? I listed numerous first-hand advantages that style
> > consistency brings and i listed numerous disadvantages created by
> > inconsistency. So i'm waiting for the list of counter-arguments -
> > there _must_ be some objective ones, besides the obvious "kernel
> > old-timers are lazy to change their ways" argument =B-)
>
> When you reject useful patches based on "this is not our preferred
> style", you piss people off. [...]

we dont actually do that for newbies and newbies are in fact happy to
write cleaner code - so the rest of your argument which depends on this
premise fails. (Most of the time i fix it up silently myself or if a
style error comes in a pattern, i ask the person to send future patches
with that small detail fixed.)

my experience with checkpatch.pl is the exact opposite of what you fear:
it _widened_ the contributor base: a good number of newbies felt
encouraged that an objective piece of tool reports an "error" in a file
that was written by otherwise "much more knowledgable" kernel hackers.
checkpatch.pl is basically the "yes, really, you are right, this piece
of code in the Linux kernel is indeed crap" review tool that reinforces
newbies. It lowers the bar of entry to kernel hacking, and it does so
for exactly those pieces of code that we want newbies to be active on:
barely maintained source code.

Whoever is afraid of an "army of checkpatch wielding newbies" who'll
never rise above their newbie status fails to consider two important
factors: 1) checkpatch errors are a finite resource to feed on and they
dont re-grow in a well-maintained subsystem 2) they need to look back a
few years when they themselves were newbies and were in need of some
easy kernel projects just to familiarize themselves with the kernel and
its contribution environment. (My first-ever contribution to the Linux
kernel was a trivial patch.)

( i only remember one patch ever being rejected due to checkpatch
failures, it came from a kernel old-timer who sent absolutely horrible
patches and who _should have known better_. Kernel old-timers are
"multipliers", they write more code and influence more people's code,
so it's expected of them to write absolutely squeaky-clean code. )

so at least for the scheduler and for arch/x86 there's absolutely zero
friction between checkpatch.pl use and newbies - and if you look at the
nicely evolving arch/x86 contributor statistics you'll have to come to
the same conclusion i believe.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/