* Theodore Tso <tytso@xxxxxxx> wrote: > On Mon, Feb 18, 2008 at 02:31:40PM +0100, Ingo Molnar wrote: > > i guess this explains what static code metrics already suggest: > > Am I right in assuming that code-quality is just a program which runs > checkpatch.pl and measures the number of warnings and calls them > errors? no, it picks the checkpatch.pl errors and calls them errors. The script ignores checkpatch.pl warnings. (although even the checkpatch.pl warnings tend to be correct in like 90% of the cases - YMMV) but yes, i'd agree with you if you said this was an arbitrary metric that does not map to the functional qualities of the code in a provable way. (other than the fact that any of these style errors are valid reasons to reject new code that is being offered into the kernel. I.e. if the ext4 codebase was submitted as a new, unknown filesystem right now, it would probably have a hard time getting accepted with all those silly style problems in it. Why should "old" subsystems have a lower threshold to meet than newer subsystems? Isnt that unfair? ) And fact is that i've yet to see really crappy code with an excellent checkpatch score, and really good code with a very poor checkpatch score. So i think you have to accept that there's _some_ correlation. And i believe this comes from the simple fact that mental carelessness is contagious: lack of attention to details on the thinking level subsconsciously slips over into the style space as well. It's hard (impossible) to measure "true" code quality, but the style space is interrelated with the "true quality" space and is easier to measure. I just had a look at the errors and warnings that checkpatch --file emits on fs/ext3/*.c, and if i was maintaining those files i'd be fixing over 50% of them - because they are just style inconsistencies often _within the same function_ which would be constant distraction when adding new code. In fact, the more critical a piece of code, the more strategic it is to users, the less acceptable it is IMO to have "style noise" and similar visual distractions in it. One unusual indentation or spacing, although silly to even mention in isolation, _can_ trick the human eye into glossing over just one critical piece of information to find or avoid a bug. And as a bonus, consistent source code style is also an attractive aesthetic experience to any first-time visitor of your subsystem. BYMMV. Ingo - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
