Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Tue, Mar 13, 2012 at 08:01:14PM -0700, Joe Perches wrote:
> > That's a debug message which is never by anyone other than ext4
> > developers.  Your patch also hacked the Makefile to enable it by
> > default,
> 
> It's just an example and no it didn't.
> That output is still in an #ifdef EXT4FS_DEBUG
> block and is unchanged.

I looked at your patch, and nearly all of them were in debug code.  I
know, because in practice the messages that come up with any kind of
regularity are all properly prefixed.

Look, the reason why I care about patch noise is because I have a huge
patch backlog.  Take a look at this:

      http://patchwork.ozlabs.org/project/linux-ext4/list/

Very few other people review patches, and even patches that survive
review, I've found problems that could potentially lead to data loss
or system instability.  This is not like your average device driver,
where if the machine panics once a week, "oh well", and you reboot.
Linus would get very cranky if he lost data as a result of a bad patch
slipping through.  Hence, patches don't go in until after significant
review and testing.

As a result, #1, patches that are don't add value, and are large,
simply won't get applied.  Period.  Avoiding the downside of lots of
people losing data is ****far**** more than your OCD wanting me to use
pr_warn(...) instead of printk(KERN_WARN, ...).

If you want your style patches to go in, break them into smaller
chunks, or I *will* ignore them.

#2, patches that don't add substantial value, and make it harder for
me to review and apply patches from my very substantial patch
backlog, I consider as adding ***substantial*** negative value.

That being said, I use checkpatch.pl as an initial screen, but it's
already been the case that there are warnings that I consider pure
noise, and I really, REALLY, rather not add more noise to
checkpatch.pl.  There is no group consensus about things like pr_foo
--- not as far as I've seen --- and to impose it from on high by some
patch wankers making changes to checkpatch.pl very much offends me.

      	      	     	     		      	   - Ted
--
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/