public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Ted Ts'o" <tytso@mit.edu>
To: Joe Perches <joe@perches.com>
Cc: Christoph Hellwig <hch@lst.de>, T Dent <tdent48227@gmail.com>,
	adilger.kernel@dilger.ca, jack@suse.cz, dmonakhov@openvz.org,
	sandeen@redhat.com, linux-ext4@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 01/10] Fs: ext4: acl.c: fixed indent issue
Date: Sat, 25 Sep 2010 21:04:11 -0400	[thread overview]
Message-ID: <20100926010410.GC19690@thunk.org> (raw)
In-Reply-To: <1285462243.6115.89.camel@Joe-Laptop>

On Sat, Sep 25, 2010 at 05:50:43PM -0700, Joe Perches wrote:
> The reason get_maintainers by default cc'd signers is mostly
> historical.  The file pattern coverage in MAINTAINERS when
> it was added wasn't very good, so signers were always added.
> It was also the Linus' preferred mechanism to find those
> "who really do the work".
> 
> http://lkml.org/lkml/2007/8/14/276

Yeah, but Linus said "subdirectory or file".  He was also assuming
some human intelligence.  In the case of a filesystem, you should use
who has done most of the work in the *subdirectory*, and not on a
file-by-file basis.  The brokeness is trying to do this in blind and
stupid script, that can't understand the difference between when you
should use subdirectory or a file.

This is what caused the completely insane result for fs/ext4/acl.c.
There have only been three commits in the past year, and they have all
been random folks who were doing random fixups --- many of which might
be stylistic cleanups.  That's not "the people who really do the
work".  It's just "the people who happened to do some random cleanups
on a file that happens not to change much".  But the stupid thing is
trying to do it on a file-by-file basis in the first place, when for
something like fs/ext4, it really should be done on a subdirectory
basis.

That, BTW, is also my biggest complaint about checkpatch.pl.  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).

					- Ted

  parent reply	other threads:[~2010-09-26  1:04 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-25 18:31 [PATCH 01/10] Fs: ext4: acl.c: fixed indent issue Tracey Dent
2010-09-25 18:31 ` [PATCH 02/10] Fs: ext4: acl.h: whitespace cleanup Tracey Dent
2010-09-26  6:11   ` Krzysztof Halasa
2010-09-25 18:31 ` [PATCH 03/10] Fs: ext: balloc: fixed a few issues that checkpatch.pl was having Tracey Dent
2010-09-25 18:31 ` [PATCH 04/10] Fs: ext4: block_validity: added space around = sign Tracey Dent
2010-09-25 18:31 ` [PATCH 05/10] Fs: ext4: ext4: cleaned up the file with checkpatch.pl Tracey Dent
2010-09-26  6:23   ` Krzysztof Halasa
2010-09-25 18:31 ` [PATCH 06/10] Fs: ext4: extents: whitespace cleanup Tracey Dent
2010-09-25 18:31 ` [PATCH 07/10] Fs: ext4: file: fixed indent problem Tracey Dent
2010-09-25 18:31 ` [PATCH 08/10] Fs: ext4: ioctl: fixed spacing issue Tracey Dent
2010-09-25 18:32 ` [PATCH 09/10] Fs: ext4: mballoc.c: whitespace cleanup Tracey Dent
2010-09-25 18:32 ` [PATCH 10/10] Fs: ext4: namei: fixed file of checkpatch/pl warnings and errors Tracey Dent
2010-09-26  6:36   ` Krzysztof Halasa
2010-09-26 18:23     ` Ted Ts'o
2010-09-26 18:35       ` Davidlohr Bueso
2010-09-26 19:17       ` Krzysztof Halasa
2010-09-25 23:36 ` [PATCH 01/10] Fs: ext4: acl.c: fixed indent issue Christoph Hellwig
2010-09-25 23:53   ` T Dent
2010-09-25 23:56     ` Christoph Hellwig
2010-09-26  0:01       ` Ted Ts'o
2010-09-26  0:09         ` Joe Perches
2010-09-26  0:32           ` Ted Ts'o
2010-09-26  0:36             ` Ted Ts'o
2010-09-26  0:50             ` Joe Perches
2010-09-26  0:58               ` Joe Perches
2010-09-26  6:42                 ` Krzysztof Halasa
2010-09-26  6:49                   ` Joe Perches
2010-09-26 11:31                     ` Krzysztof Halasa
2010-09-26  1:04               ` Ted Ts'o [this message]
2010-09-26  1:32                 ` Joe Perches
2010-09-26  1:53                   ` Ted Ts'o
2010-09-26  2:03                     ` Joe Perches
2010-09-26  2:10                       ` Ted Ts'o
2010-09-26  2:21                         ` Ted Ts'o
2010-09-26  2:45                           ` Joe Perches
2010-09-26  2:29                         ` Joe Perches
2010-09-26  0:06       ` Joe Perches
2010-09-26 20:06         ` Christoph Hellwig
2010-09-26 22:17           ` Joe Perches
2010-09-25 23:54 ` Ted Ts'o
2010-09-27 13:52   ` Eric Sandeen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100926010410.GC19690@thunk.org \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=dmonakhov@openvz.org \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=joe@perches.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=tdent48227@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox