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. > 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
