Messages in this thread | | | From | Neil Brown <> | Date | Mon, 15 Oct 2007 10:27:08 +1000 | Subject | Re: RFC: reviewer's statement of oversight |
| |
On Wednesday October 10, dgc@sgi.com wrote: > On Tue, Oct 09, 2007 at 10:49:20AM -0600, Jonathan Corbet wrote: > > Neil Brown <neilb@suse.de> wrote: > > > > + (b) Any problems, concerns, or questions relating to the patch have been > > > > + communicated back to the submitter. I am satisfied with how the > > > > + submitter has responded to my comments. > > > > > > This seems more detailed that necessary. The process (communicated > > > back / responded) is not really relevant. > > > > Instead, it seems to me that the process is crucially important. > > Reviewed-by shouldn't be a rubber stamp that somebody applies to a > > patch; I think it should really imply that issues of interest have been > > communicated to the developers. If we are setting expectations for what > > Reviewed-by means, I would prefer to leave an explicit mention of > > communication in there. > > I couldn't agree more, Jon. > > If we are to have a meaningful reviewed-by tag, it has to be clearly > documented as to what responsibilities it places on the reviewer. If > someone doesn't want to perform a well conducted review, then they > haven't earned the right to issue a Reviewed-by tag - they can use > the Acked-by rubber stamp instead.
Maybe I'm making a mountain out of a molehill but...
Clearly documented responsibilities? Yes. Prescribed process? No.
If someone sends me a patch, and I review it, and I find a couple of problems, do I need to negotiate with the submitter before correcting them and putting a "Reviewed-by" tag on it (along with my Signed-off-by before sending it upstream)?
The above clause (b) seems to say that I do. Is that something we want to mandate?
My take on the responsibilities implied by Reviewed-by: is that the code has been inspected, comprehended, considered, and found to be both appropriate and without discernible error. The process by which the code got to that state is not relevant to the tag (though it probably is relevant to the general health of the community).
NeilBrown - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |