[dpdk-dev] [PATCH] doc/contributing: add ack review descriptions

[dpdk-dev] [PATCH] doc/contributing: add ack review descriptions

[Harry van Haaren]

This commit details what is meant by the various email
tags that the DPDK community use regularly. The descriptions
state what each tag means, drawing from the kernel's understanding[1],
and the discussion on the DPDK mailing list[2].

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

[1]
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-and-cc

[2] http://dpdk.org/ml/archives/dev/2017-January/thread.html#56300
---
 doc/guides/contributing/patches.rst | 69 ++++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 9 deletions(-)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index a6b2753..2b47ab3 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -225,13 +225,9 @@ Here are some guidelines for the body of a commit message:
 
 * Use correct capitalization, punctuation and spelling.
 
-In addition to the ``Signed-off-by:`` name the commit messages can also have one or more of the following:
-
-* ``Reported-by:`` The reporter of the issue.
-* ``Tested-by:`` The tester of the change.
-* ``Reviewed-by:`` The reviewer of the change.
-* ``Suggested-by:`` The person who suggested the change.
-* ``Acked-by:`` When a previous version of the patch was acked and the ack is still relevant.
+In addition to the ``Signed-off-by:`` name the commit messages can also have
+tags for who reported, suggested, tested etc, the patch being posted. Please
+refer to section `Tested, Acked and Reviewed by`_.
 
 
 Creating Patches
@@ -427,9 +423,64 @@ The options ``--annotate`` and ``confirm = always`` are recommended for checking
 The Review Process
 ------------------
 
-The more work you put into the previous steps the easier it will be to get a patch accepted.
+Patches are reviewed by the community, relying on the experience and
+collaboration of the community to double-check each others work.
+There are a number of ways to say you have double-checked a patch on the
+mailing list.
+
+Tested, Acked and Reviewed by
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To state that you have interacted with a patch on the mailing list,
+one replies to the email with a tag. The commonly used tags are:
+
+ * Reviewed-by:
+ * Acked-by:
+ * Tested-by:
+ * Reported-by:
+ * Suggested-by:
+
+All of these tags should be used as follows:
+
+.. code-block:: console
+
+        tag-here: Name Surname <email@address.com>
+
+Each of these tags has a specific meaning. In general, the DPDK community
+generally follows the kernel usage of the tags. A short summary of the meanings
+of each tag here for reference:
+
+.. _statement: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#reviewer-s-statement-of-oversight
+
+``Reviewed-by:`` is a strong statement_ that the patch is an
+appropriate modification without any remaining serious technical issues.
+Reviewers known to understand the subject area and to perform thorough reviews
+will normally increase the likelihood of your patch getting merged.
+
+``Acked-by:`` is a record that the a person named was not directly involved in
+the preparation of a patch but wishes to signify and record their
+acceptance and approval of it.
+
+``Tested-by:`` indicates that the patch has been successfully tested (in some
+environment) by the person named.
+
+``Reported-by:`` gives credit to people who find bugs and report them and it
+hopefully inspires them to help us again in the future. Please note that if the
+bug was reported in private, then ask for permission first before using the
+Reported-by tag.
+
+``Suggested-by:`` indicates that the patch idea is suggested by the person named
+and ensures credit to the person for the idea.  Please note that this tag should
+not be added without the reporter’s permission, especially if the idea was not
+posted in a public forum.
+
+
+
+Steps to getting your patch merged
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-The general cycle for patch review and acceptance is:
+The more work you put into the previous steps the easier it will be to get a
+patch accepted. The general cycle for patch review and acceptance is:
 
 #. Submit the patch.
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] doc/contributing: add ack review descriptions

[Mcnamara, John]

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Wednesday, February 8, 2017 9:50 AM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; iryzhov@nfware.com; jonshin@cisco.com;
> Mcnamara, John <john.mcnamara@intel.com>; shreyansh.jain@nxp.com;
> Richardson, Bruce <bruce.richardson@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>
> Subject: [PATCH] doc/contributing: add ack review descriptions
> 
> This commit details what is meant by the various email tags that the DPDK
> community use regularly. The descriptions state what each tag means,
> drawing from the kernel's understanding[1], and the discussion on the DPDK
> mailing list[2].
> 
>...
>
> +``Acked-by:`` is a record that the a person named was not directly

s/the a/the/

>
> +``Suggested-by:`` indicates that the patch idea is suggested by the
> +person named and ensures credit to the person for the idea.  Please
> +note that this tag should not be added without the reporter’s

The apostrophe here is non-ascii.

Other than those minor issues:

Acked-by: John McNamara <john.mcnamara@intel.com>

[dpdk-dev] [PATCH v2] doc/contributing: add description of review tags

[Harry van Haaren]

This commit details what is meant by the various email
tags that the DPDK community use regularly. The descriptions
state what each tag means, drawing from the kernel's understanding[1],
and the discussion on the DPDK mailing list[2].

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>

[1]
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-and-cc

[2] http://dpdk.org/ml/archives/dev/2017-January/thread.html#56300
---
 doc/guides/contributing/patches.rst | 62 +++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 9 deletions(-)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index a6b2753..4fe9025 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -225,13 +225,9 @@ Here are some guidelines for the body of a commit message:
 
 * Use correct capitalization, punctuation and spelling.
 
-In addition to the ``Signed-off-by:`` name the commit messages can also have one or more of the following:
-
-* ``Reported-by:`` The reporter of the issue.
-* ``Tested-by:`` The tester of the change.
-* ``Reviewed-by:`` The reviewer of the change.
-* ``Suggested-by:`` The person who suggested the change.
-* ``Acked-by:`` When a previous version of the patch was acked and the ack is still relevant.
+In addition to the ``Signed-off-by:`` name the commit messages can also have
+tags for who reported, suggested, tested and reviewed the patch being
+posted. Please refer to the `Tested, Acked and Reviewed by`_ section.
 
 
 Creating Patches
@@ -427,9 +423,57 @@ The options ``--annotate`` and ``confirm = always`` are recommended for checking
 The Review Process
 ------------------
 
-The more work you put into the previous steps the easier it will be to get a patch accepted.
+Patches are reviewed by the community, relying on the experience and
+collaboration of the members to double-check each other's work. There are a
+number of ways to indicate that you have checked a patch on the mailing list.
+
+
+Tested, Acked and Reviewed by
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To indicate that you have interacted with a patch on the mailing list you
+should respond to the patch in an email with one of the following tags:
+
+ * Reviewed-by:
+ * Acked-by:
+ * Tested-by:
+ * Reported-by:
+ * Suggested-by:
+
+The tag should be on a separate line as follows::
+
+   tag-here: Name Surname <email@address.com>
+
+Each of these tags has a specific meaning. In general, the DPDK community
+follows the kernel usage of the tags. A short summary of the meanings of each
+tag is given here for reference:
+
+.. _statement: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#reviewer-s-statement-of-oversight
+
+``Reviewed-by:`` is a strong statement_ that the patch is an appropriate state
+for merging without any remaining serious technical issues. Reviews from
+community members who are known to understand the subject area and to perform
+thorough reviews will increase the likelihood of the patch getting merged.
+
+``Acked-by:`` is a record that the person named was not directly involved in
+the preparation of the patch but wishes to signify and record their acceptance
+and approval of it.
+
+``Tested-by:`` indicates that the patch has been successfully tested (in some
+environment) by the person named.
+
+``Reported-by:`` is used to acknowledge person who found or reported the bug.
+
+``Suggested-by:`` indicates that the patch idea was suggested by the named
+person.
+
+
+
+Steps to getting your patch merged
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-The general cycle for patch review and acceptance is:
+The more work you put into the previous steps the easier it will be to get a
+patch accepted. The general cycle for patch review and acceptance is:
 
 #. Submit the patch.
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] doc/contributing: add description of review tags

[Thomas Monjalon]

2017-02-14 11:50, Harry van Haaren:
> This commit details what is meant by the various email
> tags that the DPDK community use regularly. The descriptions
> state what each tag means, drawing from the kernel's understanding[1],
> and the discussion on the DPDK mailing list[2].
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Acked-by: John McNamara <john.mcnamara@intel.com>

Applied, thanks