From patchwork Fri Sep 22 18:37:50 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Patchwork-Submitter: Thomas Schwinge
X-Patchwork-Id: 23085
Received: (qmail 101858 invoked by alias); 22 Sep 2017 18:38:06 -0000
Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id:
List-Unsubscribe:
List-Subscribe:
List-Archive:
List-Post:
List-Help: ,
Sender: libc-alpha-owner@sourceware.org
Delivered-To: mailing list libc-alpha@sourceware.org
Received: (qmail 101812 invoked by uid 89); 22 Sep 2017 18:38:05 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-11.2 required=5.0 tests=AWL, BAYES_00,
GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE,
SPF_PASS autolearn=ham version=3.3.2 spammy=Site, casual,
acknowledgement, earn
X-Spam-User: qpsmtpd, 3 recipients
X-HELO: relay1.mentorg.com
From: Thomas Schwinge
To: Carlos O'Donell , Richard Biener
, , Gerald Pfeifer
CC: , ,
Subject: Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.
In-Reply-To: <4056e466-3055-455b-9922-55497d21fd80@redhat.com>
References: <87zi9oj8rl.fsf@euler.schwinge.homeip.net>
<347AE883-971C-447C-AB07-43F7F70F25D3@gmail.com>
<4056e466-3055-455b-9922-55497d21fd80@redhat.com>
User-Agent: Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/25.2.1
(x86_64-pc-linux-gnu)
Date: Fri, 22 Sep 2017 20:37:50 +0200
Message-ID: <87tvzuk29t.fsf@euler.schwinge.homeip.net>
MIME-Version: 1.0
Hi!
On Thu, 21 Sep 2017 12:18:39 -0600, Carlos O'Donell wrote:
> On 09/21/2017 11:56 AM, Richard Biener wrote:
> > On Thu, 21 Sep 2017 11:38:29 -0600, Carlos O'Donell wrote:
> > > On 09/21/2017 10:50 AM, Thomas Schwinge wrote:
> > > > So my question is, if I've gotten a patch reviewed by someone who is not
> > > > yet ;-) familiar with that new process, and I nevertheless want to
> > > > acknowledge their time invested in review by putting "Reviewed-by" into
> > > > the commit log, is it fine to do that if the reviewer just answered with
> > > > "OK" (or similar) instead of an explicit "Reviewed-by: NAME "
> > > > statement?
> > > You should instead ask the author to give their "Reviewed-by:" and point
> > > out what the Reviewed-by statement means.
> > >
> > > > That is, is it fine to assume that our current patch review's standard
> > > > "OK" (or similar) answer matches the more formal "Reviewer's statement of
> > > > oversight"?
> > >
> > > Not yet.
> >
> > I think given an OK from an official reviewer entitles you to commit
> > it indeed IS matching the formal statement. It better does...
I certainly understand your rationale, and do agree to that -- yet, I can
see how somebody might get offended if turning a casual "OK" into a
formal "Reviewed-by: NAME ", so...
> Isn't it better to be explicit about this; rather than assuming?
..., yeah, that makes sense.
Anyway: aside from starting to use them, we should also document such new
processes, so we might do it as follows, where I had the idea that the
*submitter* 'should encourage the reviewer to "earn" this
acknowledgement'.
Gerald, OK to commit? If approving this patch, please respond with
"Reviewed-by: NAME " so that your effort will be recorded. See
. There you go. ;-)
(I have not yet spent much time on verifying the HTML, or formatting
tweaks.)
Grüße
Thomas
Acked-by: does not necessarily indicate acknowledgement of the entire patch.
Index: htdocs/contribute.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/contribute.html,v
retrieving revision 1.87
diff -u -p -r1.87 contribute.html
--- htdocs/contribute.html 9 Apr 2015 21:49:31 -0000 1.87
+++ htdocs/contribute.html 22 Sep 2017 18:20:04 -0000
@@ -23,7 +23,7 @@ contributions must meet:
Testing Patches
Documentation Changes
Web Site Changes
-Submitting Patches
+Preparing Patches
Announcing Changes (to our Users)
@@ -164,7 +164,7 @@ file" mode of the validator.
More about our web pages.
-
+
Every patch must have several pieces of information, before we
can properly evaluate it:
@@ -257,6 +257,71 @@ bzip2ed and uuencoded or encoded as a
+
+
+
+Patch review often is a time-consuming effort. It is appreciated to
+ acknowledge this in the commit log. We are adapting
+ the Reviewed-by:
tag as established by the Linux kernel patch
+ review process.
+
+As this is not yet an established process in GCC, you, as the submitter,
+ should encourage the reviewer to "earn" this acknowledgement. For example,
+ include the following in your patch submission:
+
+
+ If approving this patch, please respond with "Reviewed-by: NAME
+ <EMAIL>" so that your effort will be recorded. See
+ <https://gcc.gnu.org/contribute.html#patches-review>.
+
+
+
+For reference, reproduced from
+ the Linux
+ kernel 4.13's Documentation/process/submitting-patches.rst
:
+
+
+
+ Reviewed-by: [...] indicates that the patch has been reviewed
+ and found acceptable according to the Reviewer's Statement:
+
+Reviewer's statement of oversight
+
+By offering my Reviewed-by: tag, I state that:
+
+ (a) I have carried out a technical review of this patch to
+ evaluate its appropriateness and readiness for inclusion [...].
+
+
+ (b) Any problems, concerns, or questions relating to the patch
+ have been communicated back to the submitter. I am satisfied
+ with the submitter's response to my comments.
+
+
+ (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 [...], and (2) free of known
+ issues which would argue against its inclusion.
+
+
+ (d) While I have reviewed the patch and believe it to be sound, I
+ do not (unless explicitly stated elsewhere) make any
+ warranties or guarantees that it will achieve its stated
+ purpose or function properly in any given situation.
+
+
+A Reviewed-by: tag is a statement of opinion that the patch is an
+appropriate modification [...] without any remaining serious
+technical issues. Any interested reviewer (who has done the work) can
+offer a Reviewed-by: tag for a patch. This tag serves to give credit to
+reviewers and to inform maintainers of the degree of review which has been
+done on the patch. Reviewed-by: tags, when supplied by reviewers known to
+understand the subject area and to perform thorough reviews, will normally
+increase the likelihood of your patch getting [...] [approved].
+
+
+Submitting Patches
+
When you have all these pieces, bundle them up in a mail message and
send it to the appropriate mailing list(s).
(Patches will go to one or more lists depending on what you are