From: Neil Brown <neilb@suse.de>
To: David Chinner <dgc@sgi.com>
Date: Mon, 15 Oct 2007 10:27:08 +1000
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Message-ID: <18194.46044.601552.537774@notabene.brown>
Cc: Jonathan Corbet <corbet@lwn.net>, Sam Ravnborg <sam@ravnborg.org>,
       linux-kernel@vger.kernel.org
Subject: Re: RFC: reviewer's statement of oversight
In-Reply-To: message from David Chinner on Wednesday October 10
References: <18186.50640.970552.719041@notabene.brown>
	<31862.1191948560@lwn.net>
	<20071010000614.GA23367404@sgi.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2281
Lines: 50

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/