On 10-12-2011 11:43, Devin Heitmueller wrote:
Hello Mauro,
On Sat, Dec 10, 2011 at 5:28 AM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxx> wrote:
Devin,
You're over-reacting. This patch were a reply from Andreas to a thread,
and not a separate patch submission.
Patches like are generally handled as RFC, especially since it doesn't
contain a description.
Any email that starts with "WTF, Devin, you again?" will probably not
get a very polite response.
I agree there's been some overreaction, but it hasn't been on my part.
He took the time to split it onto a new thread, add the subject line
"PATCH", as well as adding an SOB. Even if his intent was only to get
it reviewed, why should I waste half an hour of my time analyzing his
patch to try to figure out his intent if he isn't willing to simply
document it?
Both overacted, but this doesn't bring anything good.
You have a history of blindly accepting such patches without review.
No. I always review all patches I receive. Yeah, I have to confess:
I'm not a robot, I'm not infallible ;) (well, even a robot would
hardly be infallible, anyway).
My only intent was to flag this patch to ensure that this didn't
happen here. I've spent way more time than I should have to fixing
obscure race conditions in dvb core. If the author of a patch cannot
take the time to document his findings to provide context then the
patch should be rejected without review until he does so.
And why isn't this broken into a patch series? Even after you
analyzed the patch you still don't understand what the changes do and
why there are being made. Your explanation for why he added the
"mb()" call was because "Probably Andreas added it because he noticed
some race condition". What is the race condition? Did he find
multiple race conditions? Is this patch multiple fixes for a race
condition and some other crap at the same time?
If a developer wants a patch reviewed (as Andreas suggested was the
case here after-the-fact), then here's my feedback: break this into a
series of small incremental patches which *in detail* describe the
problem that was found and how each patch addresses the issue. Once
we have that, the maintainer can do a more in-depth analysis of
whether the patch should be accepted. Code whose function cannot be
explicitly justified but simply 'looks better' should not be mixed in
with real functional changes.
I understand that you want patches better documented, so do I, and it
would be great if this patch had a better description since the beginning.
Yet, I don't agree that this patch should be split. It does just one
thing: it fixes the timeout handling for the dvb core frontend thread.
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
