From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76046 invoked by alias); 19 Oct 2017 16:41:17 -0000 Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org Received: (qmail 75587 invoked by uid 89); 19 Oct 2017 16:41:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=peoples, people's, approvals, credit X-Spam-User: qpsmtpd, 3 recipients X-HELO: mail-qk0-f196.google.com Received: from mail-qk0-f196.google.com (HELO mail-qk0-f196.google.com) (209.85.220.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 19 Oct 2017 16:41:14 +0000 Received: by mail-qk0-f196.google.com with SMTP id l194so11093131qke.13; Thu, 19 Oct 2017 09:41:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=9zI+Qn+FFKTf5jfQ6lV5G7gYdzrM6z5e0+XKZawV4F4=; b=auUU2mI36V5OrOW9LNUYHE4yTP71IdB97Km8Qd7HIFWG+sMdwEFRQrWhDkptfR1t9+ NlDqYiHbzt/JM141FPHtejS6RbMeLRRZXRVuVfcmjX16b/PFp4xTa/Exbb1VRLSCAKxB GepyXaplB7eEkBkig1oGMXa4KW9gRB3lsUvZ50boBurHweuL/80HNRneN7rbD1QAxSeP GHD2vFyp/GCZbUBzmznEaPrX0tC3eEEQnxykfN0+dEGtqu9QVthXczoB+vdPCG6FuVNr YJkBxWrQMjlu3/HcRSYC+VWKyIp6A1V9hevMoW3uYzjpH2s2BK05OyNJ4bbG8IFvtpRQ 05vA== X-Gm-Message-State: AMCzsaXdqlIIxRbsgyqw1Pv0+BA9zGy2r1NgS8aUs1rAZZxAcKSN2GRZ 7DIMgMr/CC0roJ3WcVwp8Xpt5g== X-Google-Smtp-Source: ABhQp+R4Y+7Fj5UbXCiBEUO6ejW7jd+K90cVg0MkNvYAMX2fw2D8y5DxTSTAA1vdGj2ltJoOLMOwrA== X-Received: by 10.55.2.131 with SMTP id v3mr2590741qkg.53.1508431272042; Thu, 19 Oct 2017 09:41:12 -0700 (PDT) Received: from localhost.localdomain (71-218-5-6.hlrn.qwest.net. [71.218.5.6]) by smtp.gmail.com with ESMTPSA id t4sm1018632qtj.9.2017.10.19.09.41.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Oct 2017 09:41:11 -0700 (PDT) Subject: Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc. To: Thomas Schwinge , gcc@gcc.gnu.org, Gerald Pfeifer References: <87zi9oj8rl.fsf@euler.schwinge.homeip.net> <347AE883-971C-447C-AB07-43F7F70F25D3@gmail.com> <4056e466-3055-455b-9922-55497d21fd80@redhat.com> <87tvzuk29t.fsf@euler.schwinge.homeip.net> <87376zja8d.fsf@euler.schwinge.homeip.net> <87shefi100.fsf@euler.schwinge.homeip.net> Cc: Carlos O'Donell , Richard Biener , gdb@sourceware.org, binutils@sourceware.org, libc-alpha@sourceware.org From: Martin Sebor Message-ID: <78c896e9-7681-a29d-be41-f5c3644b6740@gmail.com> Date: Thu, 19 Oct 2017 16:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <87shefi100.fsf@euler.schwinge.homeip.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2017-10/txt/msg00060.txt.bz2 On 10/19/2017 09:57 AM, Thomas Schwinge wrote: > Hi! > > Still waiting for any kind of reaction -- general process-change inertia, > chicken-and-egg problem, I suppose. ;-/ > > I have now put the proposed text onto a wiki page, so that those > interested have a convenient handle to use, > . Quoting from the Wiki: If approving this patch, please respond with "Reviewed-by:... Often several people provide helpful feedback on patches that only one person ultimately approves. As per the GCC process, the approver is also one of the maintainers for the area affected by the patch, and so had to demonstrate the value of their own contribution to the area by committing many high quality changes of their own. Their sustained and valuable effort has already been recognized (they are prominently mentioned in the MAINTAINERs file). So without in any way diminishing their continued contribution by reviewing and approving other people's work in addition to making valuable improvements of their own, I think by focusing on approvals, the Reviewed-by proposal neglects to acknowledge the hard work of all the others who contribute to the project. Martin > > > Ping. > > On Wed, 4 Oct 2017 15:47:30 +0200, I wrote: >> Ping. >> >> On Fri, 22 Sep 2017 20:37:50 +0200, I wrote: >>> 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. ;-) >>> >>> 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.

    >>> >>> >>> -

    Submitting Patches

    >>> +

    Preparing Patches

    >>> >>>

    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 >> acceptable, as long as the ChangeLog is still posted as plain text. >>>

    >>> >>> + >>> +

    Acknowledge Patch Review

    >>> + >>> +

    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 >>> >>> (I have not yet spent much time on verifying the HTML, or formatting >>> tweaks.) > > > Grüße > Thomas >