From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: Understanding of Acked-By Date: Fri, 27 Jan 2017 12:48:06 +0530 Message-ID: <7fb35576-ba10-019a-a6ba-e38418e03848@nxp.com> References: <2358632.GCFl4gnRC2@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , "Yigit, Ferruh" , Igor Ryzhov , Steve Shin To: Thomas Monjalon , "Van Haaren, Harry" Return-path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0052.outbound.protection.outlook.com [104.47.32.52]) by dpdk.org (Postfix) with ESMTP id 5F76E282 for ; Fri, 27 Jan 2017 08:13:24 +0100 (CET) In-Reply-To: <2358632.GCFl4gnRC2@xps13> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wednesday 25 January 2017 08:28 PM, Thomas Monjalon wrote: > 2017-01-25 13:53, Van Haaren, Harry: >> There was an idea (from Thomas) to better document the Acked-by and Reviewed-By in the above thread, which I think is worth doing to make the process clearer. I'll kick off a thread*, and offer to submit a patch for the documentation when a consensus is reached. >> >> >> The question that needs to be addressed is "What is the most powerful signoff to add as somebody who checked a patch?" > > I do not see the benefit of knowing the most powerful. > Anyway, the most powerful tags are done by trusted people. > And people are trusted after delivering good reviews or patches ;) > > The question should be "How to use the tags?" > >> The documentation mentions Acked, Reviewed, and Tested by[1], as signoffs that can be commented on patches. The Review Process[2] section mentions Reviewed and Tested by, but nowhere specifically states what any of these indicate. >> >> Offered below is my current understanding of the Acked-by; Reviewed-by; and Tested-by tags, in order of least-powerful first: >> >> >> 3) Tested-by: (least powerful) >> - Indicates having passed testing of functionality, and works as expected for Tester >> - Does NOT include full code review (instead use Reviewed by) >> - Does NOT indicate that the Tester understands architecture (instead use Acked by) >> >> >> 2) Reviewed-by: >> - Indicates having passed code-review, checkpatch and compilation testing by Reviewer > > Compilation testing is done by the CI. > The reviewer must just check the results. > >> - Does NOT include full testing of functionality (instead use Tested-by) >> - Does NOT indicate that the Reviewer understands architecture (instead use Acked by) > > I disagree here. > The reviewer must understand the impacts of the patch. > That's why a Reviewed-by tag is really strong. From what I understand, 'Reviewed-by' and 'Acked-by' are the other way around. - Acked-by is intent that 'I agree with change'. - Reviewed-by is 'I vouch for the changes' either through review or testing or both. > >> 1) Acked-by: (most powerful) >> - Indicates Reviewed-by, but also: > > A maintainer may want to approve the intent without doing a full review, > especially if he trusts the author or the reviewers. > That's why I think Acked-by should not include Reviewed-by. > If a maintainer does a full review, he should use Reviewed-by instead of Acked-by. > >> - Acker understands impact to architecture (if any) and agrees with changes >> - Acker has performed runtime sanity check > > Not sure about this one. > Personnaly I give some Acks without testing sometimes. > We may add a Tested-by to indicate we made some tests. > >> - Requests "please merge" to maintainer > > Yes, "please merge" to tree maintainer (committer). > >> - Level of trust in Acked-by based on previous contributions to DPDK/networking community > > The level of trust applies to any tag or comment. > >> The above is a suggested interpretation, alternative interpretations welcomed. > > Thanks Harry >