linux-kernel - Re: [PATCH RFC 1/2] MAINTAINERS: Add "R:" designated-reviewers tag

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]

Date:	Mon, 2 Jun 2014 16:59:15 -0700
From:	josh@...htriplett.org
To:	Dave Chinner <david@...morbit.com>
Cc:	Joe Perches <joe@...ches.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	paulmck@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org,
	mingo@...nel.org, laijs@...fujitsu.com, dipankar@...ibm.com,
	akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
	niv@...ibm.com, tglx@...utronix.de, peterz@...radead.org,
	dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com,
	fweisbec@...il.com, oleg@...hat.com, sbw@....edu
Subject: Re: [PATCH RFC 1/2] MAINTAINERS: Add "R:" designated-reviewers tag

On Tue, Jun 03, 2014 at 09:19:49AM +1000, Dave Chinner wrote:
> On Mon, Jun 02, 2014 at 12:17:46PM -0700, Joe Perches wrote:
> > On Mon, 2014-06-02 at 12:09 -0700, josh@...htriplett.org wrote:
> > > On Mon, Jun 02, 2014 at 12:05:17PM -0700, Joe Perches wrote:
> > > > On Mon, 2014-06-02 at 11:55 -0700, josh@...htriplett.org wrote:
> > > > > this should go along with a change to
> > > > > get_maintainer.pl to add those folks to the CC list.
> > > > 
> > > > Something like this:
> > > 
> > > Yes, exactly.  Given an appropriate commit message,
> > > Reviewed-by: Josh Triplett <josh@...htriplett.org>
> > 
> > That's the sort of patch where reviewing is
> > pretty useless.
> > 
> > What it needs is testing, not reviewing.
> > 
> > I tested it for all of 10 seconds.
> 
> From Documentation/SubmittingPatches:
> 
> "         (c) While there may be things that could be improved with this
>              submission, I believe that it is, at this time, (1) a
>              worthwhile modification to the kernel, and (2) free of known
>              issues which would argue against its inclusion.
> .....
> 
> 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."
> 
> So, for someone to say they have reviewed the code and are able to
> say it is free of known issues and has no remaining technical
> issues, they would have had to apply, compile and test the patch,
> yes?
> 
> i.e. Reviewed-by implies both Acked-by, Tested-by and that the code
> is technically sound.

No, not at all.  It implies Acked-by, and that the code is technically
sound (both at the micro-level and in overall architecture/approach),
but does not imply Tested-by; that's a separate tag for a reason.

We should not, for instance, prevent someone from providing a
Reviewed-by (as opposed to an Acked-by) for a driver whose hardware few
people actually have.  There's significant value in code review even
without the ability to test.

> Anyone using Reviewed-by without having actually applied and tested
> the patch is mis-using the tag - they should be using Acked-by: if
> all they have done is read the code in their mail program....

Acked-by and Reviewed-by mean two different things (Reviewed-by being a
superset of Acked-by), and the difference is not "I've applied and
tested this"; that's Tested-by.

- Josh Triplett
--
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/