Messages in this thread | | | Date | Wed, 14 Mar 2012 08:34:29 -0400 | From | Ted Ts'o <> | Subject | Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL> |
| |
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
| |