On Mon, Sep 27, 2010 at 4:06 PM, Greg Freemyer <greg.freemyer@xxxxxxxxx> wrote
All,
The last couple of days on the ext4 list there has been some
discussion of ./scripts/checkpatch.pl and ./scripts/get_maintainer.pl.
It looks like a simple whitespace patch is going to be rejected. (surpise!)
If you haven't seen it, I think it is worth a read. Especially if
you're a newbie. (As I assume the originator of the thread was.)
Thanks for this, Greg - much appreciated. This really was interesting to follow. I thought the following
comment from Ted Ts'o summed the discussion up nicely:
If it's used by newbies who want to get warned about obvious things, that's fine.
If it's used by maintainers as an automated way to catch nits, that's also fine.
Maintainers are experts who know when it's OK to disregard flase positives.What really annoys me is newbies who use checkpatch.pl in its --file mode,and then assume that every single warning is a deadly bug that much be patched.Scripts by definitions are stupid, and don't substitute for thinking. checkpatch.pl at least as the excuse that it has some valid non-stupid uses. But I'm not convinced get_maintainers.pl has the same excuse.
I at least never use it. I'll look through the MAINTAINERS file by hand, or I'll use git log by hand, and let my human intelligence figure out whether or not the patches that are turned up constitute "those that do real work", or are bullshit checkpatch.pl cleanup patches. Training people to use a script that by defintion can't be smart enough to make these distinction ultimately is a huge disservice to newbies (and experts won't use get_maintinaer.pl anyway, because they will want to know the context).
I'm following the drivers mailing list trying to get a feel for protocol at the moment
and one of the things that really surprised me was that there were patches being sent
for pure code tidy-up re-formats (i.e. not as part of actually fixing bugs, optimizing code etc.).
I had previously read Jon Corbet's 'How to Participate in the Linux Community -
a Guide to the Development Process' where he made the following point:
Developers may
start to generate reformatting patches as a way of gaining familiarity
with the process, or as a way of getting their name into the kernel
changelogs – or both. But pure coding style fixes are seen as noise
by the development community; they tend to get a chilly reception.
So this type of patch is best avoided. It is natural to fix the style of a
piece of code while working on it for other reasons, but coding style
changes should not be made for their own sake.
The ext4 list discussion seems to indicate that this is still good advice for would-be
contributors to follow... :-)
(I'm also impressed by the patience and diplomatic attitudes of maintainers like Greg KH and Ted Ts'o who presumably must keep getting sent stuff like this. :-) )
Thanks,
Julie
