Re: [RFH] How to review patches: Documentation/ReviewingPatches?

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Johannes Schindelin
Date: Thursday, February 12, 2009 - 5:08 pm

Hi,

On Fri, 13 Feb 2009, Jakub Narebski wrote:


As I found out quite painfully recently, reviewing is a matter of trust, 
much more so than a matter of form or style.

I mean, it does not really hurt when somebody says ACK not understanding 
that an ACK is expected to come from people who wrote that particular 
code, or are at least very familiar with it.  We know what the guy means, 
and that's it.

However, it does hurt when somebody says "I tested this extensively, and 
it works, I also reviewed the code, and it is correct" and you find out 
much later that the review consisted of a run through "indent" and seeing 
that there were no differences.  Unsurprisingly, the patch had to be 
reverted entirely.

It's much better to have somebody prove her capability as an excellent 
reviewer, for example by offering comments that result in a better patch, 
earning respect by others in the process.

Speaking for myself, it is also quite possible that you find out that your 
reviewing fu is leaving to be desired.  Concretely, it is widely known 
that patches I reviewed still contain several issues after I find no more.

But at least I can serve as an early filter as long as I have the time.

There is another reason why I do not want any ReviewingPatches: reviewing 
is already such a tedious process; let's not make it harder by forcing a 
potential reviewer to sift through a document (the same could be said 
about SubmittingPatches; IMHO it just repeats what common sense would do 
anyway when imitating existing code).

I'd rather suggest to patch submitters to make such a good case that all 
the world is interested in their patch, throwing a lot of eyeballs (AKA 
review) at it.

BTW this is a pretty valuable skill, also (maybe especially) outside of 
this mailing list, to get people interested in your work.

Ciao,
Dscho

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [RFH] How to review patches: Documentation/ReviewingPa ..., Johannes Schindelin, (Thu Feb 12, 5:08 pm)
Re: [RFH] How to review patches: Documentation/ReviewingPa ..., Marius Storm-Olsen, (Fri Feb 13, 12:54 am)
Re: [RFH] How to review patches: Documentation/ReviewingPa ..., Johannes Schindelin, (Fri Feb 13, 4:05 am)