Hi! Ping.
On Fri, 22 Sep 2017 20:37:50 +0200, I wrote: > On Thu, 21 Sep 2017 12:18:39 -0600, Carlos O'Donell <car...@redhat.com> wrote: > > On 09/21/2017 11:56 AM, Richard Biener wrote: > > > On Thu, 21 Sep 2017 11:38:29 -0600, Carlos O'Donell <car...@redhat.com> > > > 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 <EMAIL>" > > > > > 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 <EMAIL>", 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 <EMAIL>" so that your effort will be recorded. See > <https://gcc.gnu.org/contribute.html#patches-review>. There you go. ;-) > > 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:</p> > <li><a href="#testing">Testing Patches</a></li> > <li><a href="#docchanges">Documentation Changes</a></li> > <li><a href="#webchanges">Web Site Changes</a></li> > -<li><a href="#patches">Submitting Patches</a></li> > +<li><a href="#patches">Preparing Patches</a></li> > <li><a href="#announce">Announcing Changes (to our Users)</a></li> > </ul> > > @@ -164,7 +164,7 @@ file" mode of the validator.</p> > <p>More <a href="about.html#cvs">about our web pages</a>.</p> > > > -<h2><a name="patches">Submitting Patches</a></h2> > +<h2><a name="patches">Preparing Patches</a></h2> > > <p>Every patch must have several pieces of information, <em>before</em> we > can properly evaluate it:</p> > @@ -257,6 +257,71 @@ bzip2ed and uuencoded or encoded as a <c > acceptable, as long as the ChangeLog is still posted as plain text. > </p> > > +<!-- (Eventually) referenced from many places. --> > +<h3><a name="patches-review">Acknowledge Patch Review</a></h3> > + > +<p>Patch review often is a time-consuming effort. It is appreciated to > + acknowledge this in the commit log. We are adapting > + the <code>Reviewed-by:</code> tag as established by the Linux kernel patch > + review process.</p> > + > +<p>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:</p> > + > +<blockquote> > + <p>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>. > + </p> > +</blockquote> > + > +<p>For reference, reproduced from > + the <a > href="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v4.13#n560">Linux > + kernel 4.13's > <code>Documentation/process/submitting-patches.rst</code></a>: > +</p> > + > +<blockquote > cite="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v4.13#n560"> > + <p><em>Reviewed-by:</em> [...] indicates that the patch has been reviewed > + and found acceptable according to the Reviewer's Statement:<br> > +<br> > +<strong>Reviewer's statement of oversight</strong><br> > +<br> > +By offering my <em>Reviewed-by:</em> tag, I state that:<br> > +<br> > + (a) I have carried out a technical review of this patch to > + evaluate its appropriateness and readiness for inclusion [...]. > +<br> > +<br> > + (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. > +<br> > +<br> > + (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. > +<br> > +<br> > + (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. > +<br> > +<br> > +A <em>Reviewed-by:</em> 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 <em>Reviewed-by:</em> 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. <em>Reviewed-by:</em> 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]. > +</p></blockquote> > + > +<h3>Submitting Patches</a></h3> > + > <p>When you have all these pieces, bundle them up in a mail message and > send it to <a href="lists.html">the appropriate mailing list(s)</a>. > (Patches will go to one or more lists depending on what you are > > (I have not yet spent much time on verifying the HTML, or formatting > tweaks.) Grüße Thomas