linux-kernel - Re: RFC: reviewer's statement of oversight

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:  <20071008203429.7c37ee03@freepuppy.rosehill>
Date:	Mon, 8 Oct 2007 20:34:29 -0700
From:	Stephen Hemminger <shemminger@...ux-foundation.org>
To:	linux-kernel@...r.kernel.org
Subject:  Re: RFC: reviewer's statement of oversight

On Mon, 8 Oct 2007 16:06:03 -0700
Randy Dunlap <randy.dunlap@...cle.com> wrote:

> On Mon, 08 Oct 2007 16:43:10 -0600 Jonathan Corbet wrote:
> 
> > Sam Ravnborg <sam@...nborg.org> wrote:
> > 
> > > Or maybe we need something much less formal that explain the purpose of the
> > > four tags we use:
> > 
> > ...or maybe a combination?  How does the following patch look as a way
> > to describe how the tags are used and what Reviewed-by, in particular,
> > means?
> > 
> > Perhaps the DCO should move to this file as well?
> > 
> > jon
> 
> Just typos noted below...
> 
> > ---
> > 
> > Add a document on patch tags.
> > 
> > Signed-off-by: Jonathan Corbet <corbet@....net>
> > 
> > diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
> > index 43e89b1..fa1518b 100644
> > --- a/Documentation/00-INDEX
> > +++ b/Documentation/00-INDEX
> > @@ -284,6 +284,8 @@ parport.txt
> >  	- how to use the parallel-port driver.
> >  parport-lowlevel.txt
> >  	- description and usage of the low level parallel port functions.
> > +patch-tags
> > +	- description of the tags which can be added to patches
> >  pci-error-recovery.txt
> >  	- info on PCI error recovery.
> >  pci.txt
> > diff --git a/Documentation/patch-tags b/Documentation/patch-tags
> > new file mode 100644
> > index 0000000..fb5f8e1
> > --- /dev/null
> > +++ b/Documentation/patch-tags
> > @@ -0,0 +1,66 @@
> > +Patches headed for the mainline may contain a variety of tags documenting
> > +who played a hand in (or was at least aware of) its progress.  All of these
> > +tags have the form:
> > +
> > +	Something-done-by: Full name <email@...ress>
> > +
> > +These tags are:
> > +
> > +Signed-off-by:  A person adding a Signed-off-by tag is attesting that the
> > +		patch is, to the best of his or her knowledge, legally able
> > +		to be merged into the mainline and distributed under the
> > +		terms of the GNU General Public License, version 2.
All changes are licensed under the terms of the file modified. 

(Some people seem not to understand that
if the file is dual licensed, then the changes are dual licensed. 
If file is GPL v2 only, then the changes are GPL v2 only, ...)

> >  See
> > +		the Developer's Certificate of Origin, found in
> > +		Documentation/SubmittingPatches, for the precise meaning of
> > +		Signed-off-by.


> > +Acked-by:	The person named (who should be an active developer in the
> > +		area addressed by the patch) is aware of the patch and has
> > +		no objection to its inclusion.  An Acked-by tag does not
> > +		imply any involvement in the development of the patch or
> > +		that a detailed review was done.
> > +
> > +Reviewed-by:	The patch has been reviewed and found acceptible according
> 
>                                                       acceptable
> 
> > +		to the Reviewer's Statement as found at the bottom of this
> > +		file.  A Reviewed-by tag is a statement of opinion that the
> > +		patch is an appropriate modification of the kernel without
> > +		any remaining serious technical issues.  Any interested
> > +		reviewer (who has done the work) can offer a Reviewed-by
> > +		tag for a patch.
> > +
> > +Cc:		The person named was given the opportunity to comment on
> > +		the patch.  This is the only tag which might be added
> > +		without an explicit action by the person it names.
> > +
> > +Tested-by:	The patch has been successfully tested (in some
> > +		environment) by the person named.
> > +
>

IMHO the other tags actually are a poor substitute for providing a
more complete description of the reviewer's involvement. It would be better
to have more complete responses like "the patch should be merged as is for
2.6.X but the following should be fixed, ..." etc. The certificate of origin
has meaning for legal things that have a more concrete definition, but the
existing process is about people making good (or bad) decisions based on
feedback and other data. Trying to reduce the feedback down to 3 Acks, and 1 Review
seems like noise. The problem is getting good reviews of new code in
a timely manner, not the descriptions of the result.


-- 
Stephen Hemminger <shemminger@...ux-foundation.org>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/