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

From: Ingo Molnar
Date: Tue Mar 25 2008 - 08:24:52 EST



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

> On Tue, 25 March 2008 11:48:41 +0100, Ingo Molnar wrote:
> >
> > So, in the specific example of the scheduler subsystem, i've only
> > observed advantages to checkpatch and zero downsides. Could anyone
> > give me _any_ objective reason why i shouldnt be using checkpatch
> > for the scheduler? More broadly, could anyone give me an objective
> > reason why we shouldnt be doing it for arch/x86? And even more
> > broadly, could anyone give me an objective reason why we shouldnt be
> > doing it for all actively maintained areas of the kernel?
>
> Disagreement between checkpatch and maintainers preferred style. I've
> had a patch that fixed a bug and - while in the region - "cleaned up"
> the style for a single line. This line no longer matches the rest of
> the file and creates the kind of visual distraction you complain
> about.

well, once a subsystem is "checkpatch-clean" (which my challenge above
obviously assumes), there's no such "partial style" problem. It
obviously also assumes that the maintainer agrees that having consistent
coding style across all of Linux is a long-term advantage.

The current visual inconsistency between subsystems makes the Linux
kernel appear rather unpleasant and unprofessional to new kernel
developers. This is not just embarrasing to us (we want to write the
best OS on the planet), it is also actively harmful: such a "consistent
style does not matter" stance turns away people who have taste and tends
to attract people who have no taste - which i'm sure you'll agree with
results in a deadly spiral if it gets strong enough.

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-)

In my experience in almost all cases the "style disagreement" between a
subsystem maintainer and checkpatch is due to the _maintainer_ being
wrong about seemingly unimportant (to him) style details.

And that very much includes myself: i used to have such "disagreement"
with checkpatch errors and i used to be annoyed at the style nitpicking
of checkpatch. And yes, it takes a certain amount of time for a
long-time kernel hacker like me to realize that a lot of code i wrote in
the past needs a good clean-up ;-)

These style differences are certainly not "wrong enough" to
inconvenience or displace an active maintainer (and i never made that
point), but they are nevertheless a death by a thousand cuts that the
general kernel is suffering from right now, and i'd be a fool not to
point it out. These seemingly unimportant style details add up to a
hodgepodge of coding style that makes life difficult for people who have
to look at many different parts of the kernel that they _dont maintain_.

That's why i asked about specific examples that we can talk about - and
i gave specific examples and filenames. The checkpatch maintainer (Andy
Whitcroft) has certainly shown flexibility to fix false positives ASAP.

> Of course when a maintainer likes checkpatch, as you do, there is no
> disagreement to deal with. :)

note, all those patches are "Subject: x86: " and 99% of them is
maintained by us x86 maintainers.

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/