* [dpdk-dev] [PATCH] add a dpdk contributors guide
@ 2015-10-15 16:51 John McNamara
2015-10-15 16:51 ` [dpdk-dev] [PATCH] doc: add " John McNamara
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: John McNamara @ 2015-10-15 16:51 UTC (permalink / raw)
To: dev
Add a document to explain the DPDK patch submission and review process.
This is based on the existing information on the http://dpdk.org/dev
webpage, on the Linux kernel patch submission guidelines, and on the
existing day to day workflow on the mailing list.
This is a draft so feel free to chime in with additions and corrections.
John McNamara (1):
doc: add contributors guide
doc/guides/contributing/index.rst | 1 +
doc/guides/contributing/patches.rst | 309 ++++++++++++++++++++++++++++++++++++
2 files changed, 310 insertions(+)
create mode 100644 doc/guides/contributing/patches.rst
--
1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH] doc: add contributors guide
2015-10-15 16:51 [dpdk-dev] [PATCH] add a dpdk contributors guide John McNamara
@ 2015-10-15 16:51 ` John McNamara
2015-10-15 21:36 ` Thomas Monjalon
2015-10-20 11:03 ` [dpdk-dev] [PATCH v2] " John McNamara
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: John McNamara @ 2015-10-15 16:51 UTC (permalink / raw)
To: dev
Add a document to explain the DPDK patch submission and review process.
Signed-off-by: John McNamara <john.mcnamara@intel.com>
---
doc/guides/contributing/index.rst | 1 +
doc/guides/contributing/patches.rst | 309 ++++++++++++++++++++++++++++++++++++
2 files changed, 310 insertions(+)
create mode 100644 doc/guides/contributing/patches.rst
diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index 561427b..f49ca88 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -9,3 +9,4 @@ Contributor's Guidelines
design
versioning
documentation
+ patches
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
new file mode 100644
index 0000000..e5d28d5
--- /dev/null
+++ b/doc/guides/contributing/patches.rst
@@ -0,0 +1,309 @@
+.. submitting_patches:
+
+Contributing Code to DPDK
+=========================
+
+This document outlines the guidelines for submitting code to DPDK.
+
+The DPDK development process is modeled (loosely) on the Linux Kernel development model so it is worth reading the
+Linux kernel guide on submitting patches:
+`How to Get Your Change Into the Linux Kernel <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_.
+The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
+
+
+The DPDK Development Process
+-----------------------------
+
+The DPDK development process has the following features:
+
+* The code is hosted in a public git repository.
+* There is a mailing list where developers submit patches.
+* There are maintainers for hierarchical components.
+* Patches are reviewed publicly on the mailing list.
+* Successfully reviewed patches are merged to the master branch of the repository.
+
+The mailing list for DPDK development is `dev@dpkg.org <http://dpdk.org/ml/archives/dev/>`_.
+Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
+It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwxispork/project/dpdk/list/>`_
+
+There are also DPDK mailing lists for:
+
+* users: `general usage questions <http://dpdk.org/ml/listinfo/users>`_.
+* announce: `release announcements <http://dpdk.org/ml/listinfo/announce>`_ (also forwarded to the dev list).
+* dts: `test suite reviews and discussions <http://dpdk.org/ml/listinfo/dts>`_.
+* test-reports: `test reports <http://dpdk.org/ml/listinfo/test-report>`_ (from continuous integration testing).
+
+The development process requires some familiarity with the ``git`` version control system.
+Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
+
+
+Getting the Source Code
+-----------------------
+
+The source code can be cloned using either of the following::
+
+ git clone git://dpdk.org/dpdk
+
+ git clone http://dpdk.org/git/dpdk
+
+You can also `browse the source code <http://www.dpdk.org/browse/dpdk/tree/>`_ online.
+
+
+Make your Changes
+-----------------
+
+Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
+
+* Follow the :ref:`coding_style` guidelines.
+
+* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
+
+* If your changes add new external functions then they should be added to the local ``version.map`` file.
+ See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
+
+* Most changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
+ See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
+
+* Don’t break compilation between commits with forward dependencies.
+ Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
+
+* Add tests to the the ``app/test`` unit test framework where possible.
+
+* Add documentation, if required, in the form of Doxygen comments or a User Guide in RST format.
+ See the :ref:`Documentation Guidelines <doc_guidelines>`.
+
+Once the changes have been made you should commit them to your local repo.
+The commits should be separated into logical patches in a patchset.
+In general commits should be separated based on their directory such as ``lib``, ``drivers``, ``scripts`` although
+some of these, such as ``drivers`` may require finer grained separation.
+The easiest way of determining this is to do a ``git log`` on changed or similar files.
+
+Example of a logical patchset separation::
+
+ [patch 1/6] ethdev: add support for ieee1588 timestamping
+ [patch 2/6] e1000: add support for ieee1588 timestamping
+ [patch 3/6] ixgbe: add support for ieee1588 timestamping
+ [patch 4/6] i40e: add support for ieee1588 timestamping
+ [patch 5/6] app/testpmd: refactor ieee1588 forwarding
+ [patch 6/6] doc: document ieee1588 forwarding mode
+
+
+The component separation will also be used in the subject line of the commit message, see below.
+The required format of the commit messages is shown in the next sections.
+
+
+Commit Messages: Subject Line
+-----------------------------
+
+The first, summary, line of the git commit message becomes the subject line of the patch email.
+Here are some guidelines for the summary line:
+
+* The summary line should be around 50 characters.
+
+* The summary line should be lowercase.
+
+* It should be prefixed with the component name (use git log to check existing components).
+ For example::
+
+ config: enable same drivers options for linux and bsd
+
+ ixgbe: fix offload config option name
+
+* Use the imperative of the verb (like instructions to the code base).
+ For example::
+
+ ixgbe: fix bug in xyz
+
+ ixgbe: add refcount to foo struct
+
+* Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
+
+The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
+for example: ``PATCH v2``.
+The is generally added by ``git send-email`` or ``git format-patch``, see below.
+
+If you are submitting a RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
+
+
+Commit Messages: Body
+---------------------
+
+Here are some guidelines for the body of a commit message:
+
+* You must provide a body to the commit message after the subject/summary line.
+ Do not leave it blank.
+
+* The body of the message should describe the issue being fixed or the feature being added.
+ It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
+
+* The commit message must end with a ``Signed-off-by:`` line which is added using::
+
+ git commit --signoff # or -s
+
+ The purpose of the signoff is explained in the
+ `Developer's Certificate of Origin <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_
+ section of the Linux kernel guidelines.
+
+ .. Note::
+
+ All developers must ensure that they have read and understood the
+ Developer's Certificate of Origin section of the documentation prior
+ to applying the signoff and submitting a patch.
+
+* The signoff must be a real name and not an alias or nickname.
+ More than one signoff is allowed.
+
+* The text of the commit message should be wrapped at 72 characters.
+
+* When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
+ You can generate the required text as follows::
+
+ git log -1 COMMIT_ID --abbrev=12 --format='Fixes: %h ("%s")'
+
+ Fixes: a4024448efa6 ("i40e: add ieee1588 timestamping")
+
+* When fixing an error or warning it is useful to add the error message or output.
+
+* 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.
+
+
+Creating Patches
+----------------
+
+It is possible to send patches directly from git but for new contributors it is recommended to generate the
+patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
+send them with ``git send-mail``.
+
+Here are some examples of using ``git format-patch`` to generate patches:
+
+.. code-block:: console
+
+ # Generate a patch from the last commit.
+ git format-patch -1
+
+ # Generate a patch from the last 3 commits.
+ git format-patch -3
+
+ # Generate the patches in a directory.
+ git format-patch -3 -o ~/patch/
+
+ # Add a cover letter to explain a patchset.
+ git format-patch -3 -o --cover-letter
+
+ # Add a prefix with a version number.
+ git format-patch -3 -o --subject-prefix 'PATCH v2'
+
+
+Cover letters are useful for explaining a patchset.
+Smaller notes can be put inline in the patch after the ``---`` separator, for example::
+
+ Subject: [PATCH] fm10k/base: add FM10420 device ids
+
+ Add the device ID for Boulder Rapids and Atwood Channel to enable
+ drivers to support those devices.
+
+ Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
+ ---
+
+ ADD NOTES HERE.
+
+ drivers/net/fm10k/base/fm10k_api.c | 6 ++++++
+ drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
+ 2 files changed, 12 insertions(+)
+ ...
+
+Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
+This can go either in the cover letter on the annotations.
+For example::
+
+ v3:
+ * Fixed issued with version.map.
+
+ v2:
+ * Added i40e support.
+ * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
+ since 802.1AS can be supported through the same interfaces.
+
+
+Checking the Patches
+--------------------
+
+Patches should be checked for formatting and syntax issues using the Linux scripts tool ``checkpatch``.
+
+The ``checkpatch`` utility can be obtained by cloning, and periodically, updating the Linux kernel sources.
+
+The kernel guidelines that are tested by ``checkpatch`` don't match the DPDK Coding Style guidelines exactly but
+they provide a good indication of conformance.
+Warnings about not using kernel data types or about split strings can be ignored::
+
+ /path/checkpatch.pl --ignore PREFER_KERNEL_TYPES,SPLIT_STRING -q files*.patch
+
+
+Sending Patches
+---------------
+
+Patches should be sent to the mailing list using ``git send-email``.
+This will require a working and configured ``sendmail`` or similar application.
+See the `Git send-mail <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
+
+The patches should be sent to ``dev@dpdk.org``::
+
+ git send-email --to dev@dpdk.org 000*.patch
+
+If the patches are a change to existing files then you should CC the maintainer(s) of the changed files.
+The appropriate maintainer can be found in the ``MAINTAINERS`` file::
+
+ git send-email --to dev@dpdk.org --cc maintainer@some.org 000*.patch
+
+You can test the emails by sending it to yourself or with the ``--dry-run`` option.
+
+If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
+
+ git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
+
+The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
+`for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
+
+
+Once submitted your patches will appear on the mailing list and in Patchwork.
+
+Experienced commiters may send patches directly with ``git send-email`` without the ``git format-patch`` step.
+
+
+The Review Process
+------------------
+
+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.
+
+#. Wait for review comments. While you are waiting review some other patches.
+
+#. Fix the review comments and submit a ``v n+1`` patchset::
+
+ git format-patch -3 -o --subject-prefix 'PATCH v2'
+
+#. Update Patchwork to mark your previous patches as "Superseded".
+
+#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
+ the patch with an email that includes something like::
+
+ Acked-by: Alex Smith <alex.smith@example.com>
+
+ **Note**: When acking patches please remove as much of the text of the patch email as possible.
+ It is generally best to delete everything after the ``Signed-off-by:`` line.
+
+#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
+ it may receive a ``nack``.
+ In this case you will need to make a more convincing technical argument in favor of your patches.
+
+#. Acked patches will be merged in the next merge window.
--
1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] doc: add contributors guide
2015-10-15 16:51 ` [dpdk-dev] [PATCH] doc: add " John McNamara
@ 2015-10-15 21:36 ` Thomas Monjalon
2015-10-20 10:58 ` Mcnamara, John
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2015-10-15 21:36 UTC (permalink / raw)
To: John McNamara; +Cc: dev
Hi John,
2015-10-15 17:51, John McNamara:
> Add a document to explain the DPDK patch submission and review process.
Thanks
> +There are also DPDK mailing lists for:
> +
> +* users: `general usage questions <http://dpdk.org/ml/listinfo/users>`_.
> +* announce: `release announcements <http://dpdk.org/ml/listinfo/announce>`_ (also forwarded to the dev list).
> +* dts: `test suite reviews and discussions <http://dpdk.org/ml/listinfo/dts>`_.
I think these lists are not relevant for patch submission.
> +* test-reports: `test reports <http://dpdk.org/ml/listinfo/test-report>`_ (from continuous integration testing).
[...]
> +Getting the Source Code
> +-----------------------
> +
> +The source code can be cloned using either of the following::
> +
> + git clone git://dpdk.org/dpdk
> +
> + git clone http://dpdk.org/git/dpdk
> +
> +You can also `browse the source code <http://www.dpdk.org/browse/dpdk/tree/>`_ online.
The online browse doesn't help for patch contribution.
[...]
> +* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
yes
> +* If your changes add new external functions then they should be added to the local ``version.map`` file.
> + See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
> +
> +* Most changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
> + See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
s/Most/Important/ ?
> +* Don’t break compilation between commits with forward dependencies.
> + Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
no, please, don't break compilation :)
> +* Add tests to the the ``app/test`` unit test framework where possible.
> +
> +* Add documentation, if required, in the form of Doxygen comments or a User Guide in RST format.
s/required/relevant/ ?
> +The commits should be separated into logical patches in a patchset.
Yes
> +In general commits should be separated based on their directory such as ``lib``, ``drivers``, ``scripts`` although
> +some of these, such as ``drivers`` may require finer grained separation.
No. The directory is not so important.
It must be easy to review first.
If changes are not so big and do not require specific explanations,
it's better to keep things together in the same patch.
A good way of thinking about patch split is to consider backports:
will it be easy to backport this change with its dependencies?
will it be easy to backport this feature/fix without useless bloat?
> +The easiest way of determining this is to do a ``git log`` on changed or similar files.
Yes
> +Example of a logical patchset separation::
> +
> + [patch 1/6] ethdev: add support for ieee1588 timestamping
> + [patch 2/6] e1000: add support for ieee1588 timestamping
> + [patch 3/6] ixgbe: add support for ieee1588 timestamping
> + [patch 4/6] i40e: add support for ieee1588 timestamping
> + [patch 5/6] app/testpmd: refactor ieee1588 forwarding
> + [patch 6/6] doc: document ieee1588 forwarding mode
The doc must be committed with the API change (ethdev).
Splitting driver implementations is useful only if they are really big or
require some specific explanations in the commit message.
> +* The summary line should be lowercase.
The acronyms can be uppercase.
> + For example::
> +
> + ixgbe: fix bug in xyz
After "fix", the word "bug" is useless.
It's better to briefly explain the impact of the bug, e.g. "fix RSS on 32-bit".
So people interested in RSS or 32-bit will look at this fix.
> + ixgbe: add refcount to foo struct
Generally, using the name of a struct, a variable or a file in the title
reveals that you don't know how to explain your change simply.
The implementation details may be explained in the long message.
The title must help to catch the area and the impact of the change.
> +If you are submitting a RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
A RFC may be incomplete.
It helps to have feedbacks before doing more.
> +* You must provide a body to the commit message after the subject/summary line.
> + Do not leave it blank.
When it is totally obvious, the Signed-off is enough.
> +* When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
> + You can generate the required text as follows::
> +
> + git log -1 COMMIT_ID --abbrev=12 --format='Fixes: %h ("%s")'
git alias: fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'
> + Fixes: a4024448efa6 ("i40e: add ieee1588 timestamping")
Yes it will help the backports.
> +* When fixing an error or warning it is useful to add the error message or output.
The steps to reproduce the bugs are also required.
> +* ``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.
Yes, and Acked-by:
When it is commented between 2 versions of the patch, it can be added in the
new version if it is still relevant.
> +Cover letters are useful for explaining a patchset.
And it helps to have a correct threading of the patches.
> +Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
> +This can go either in the cover letter on the annotations.
s/on/or/
> +The kernel guidelines that are tested by ``checkpatch`` don't match the DPDK Coding Style guidelines exactly but
> +they provide a good indication of conformance.
> +Warnings about not using kernel data types or about split strings can be ignored::
> +
> + /path/checkpatch.pl --ignore PREFER_KERNEL_TYPES,SPLIT_STRING -q files*.patch
OK
I plan to suggest a script with more checkpatch configurations.
We should enforce using "make test" before sending.
> +Patches should be sent to the mailing list using ``git send-email``.
> +This will require a working and configured ``sendmail`` or similar application.
No, you can configure an external SMTP:
smtpuser = name@domain.com
smtpserver = smtp.domain.com
smtpserverport = 465
smtpencryption = ssl
> +If the patches are a change to existing files then you should CC the maintainer(s) of the changed files.
> +The appropriate maintainer can be found in the ``MAINTAINERS`` file::
> +
> + git send-email --to dev@dpdk.org --cc maintainer@some.org 000*.patch
I would say to send --to the maintainers and -cc dev@dpdk.org.
Some maintainers can have stronger filter if their name is in the "To" field.
> +If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
> +
> + git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
Yes please.
s/can/should/
> +Experienced commiters may send patches directly with ``git send-email`` without the ``git format-patch`` step.
The options --annotate and "confirm = always" are recommended to check before sending.
> +The more work you put into the previous steps the easier it will be to get a patch accepted.
Yes :)
> +#. Submit the patch.
Check the automatic test reports in the coming hours.
> +#. Wait for review comments. While you are waiting review some other patches.
> +#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
> + the patch with an email that includes something like::
We don"t use Reviewed-by a lot.
My understanding is that "Acked-by" doesn't mean it has been fully reviewed and tested.
But Reviewed-by is stronger without implying that we think it's the best solution.
It's an interpretation. Should it be explained here?
> +#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
> + it may receive a ``nack``.
> + In this case you will need to make a more convincing technical argument in favor of your patches.
More generally, a patch should not be accepted if there are some comments not
addressed by a new version or some strong arguments.
> +#. Acked patches will be merged in the next merge window.
Next or current?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] doc: add contributors guide
2015-10-15 21:36 ` Thomas Monjalon
@ 2015-10-20 10:58 ` Mcnamara, John
0 siblings, 0 replies; 14+ messages in thread
From: Mcnamara, John @ 2015-10-20 10:58 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, October 15, 2015 10:37 PM
> To: Mcnamara, John
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] doc: add contributors guide
>
> Hi John,
>
> 2015-10-15 17:51, John McNamara:
> > Add a document to explain the DPDK patch submission and review process.
>
> Thanks
>
> > +There are also DPDK mailing lists for:
> > +
> > +* users: `general usage questions
> <http://dpdk.org/ml/listinfo/users>`_.
> > +* announce: `release announcements
> <http://dpdk.org/ml/listinfo/announce>`_ (also forwarded to the dev list).
> > +* dts: `test suite reviews and discussions
> <http://dpdk.org/ml/listinfo/dts>`_.
>
> I think these lists are not relevant for patch submission.
Hi Thomas,
I've addressed your suggestions and will submit a v2.
I still have some uncertainty around the process for patch split and ack/nack/merge. If you have better suggestions let me know.
John.
--
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2] doc: add contributors guide
2015-10-15 16:51 [dpdk-dev] [PATCH] add a dpdk contributors guide John McNamara
2015-10-15 16:51 ` [dpdk-dev] [PATCH] doc: add " John McNamara
@ 2015-10-20 11:03 ` John McNamara
2015-10-23 10:18 ` [dpdk-dev] [PATCH v3] " John McNamara
2015-11-20 12:46 ` [dpdk-dev] [PATCH v4] " John McNamara
3 siblings, 0 replies; 14+ messages in thread
From: John McNamara @ 2015-10-20 11:03 UTC (permalink / raw)
To: dev
Add a document to explain the DPDK patch submission and review process.
Signed-off-by: John McNamara <john.mcnamara@intel.com>
---
v2:
* Fixes for mailing list comments.
* Fix for broken link target.
doc/guides/contributing/documentation.rst | 2 +-
doc/guides/contributing/index.rst | 1 +
doc/guides/contributing/patches.rst | 327 ++++++++++++++++++++++++++++++
3 files changed, 329 insertions(+), 1 deletion(-)
create mode 100644 doc/guides/contributing/patches.rst
diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
index 7c1eb41..0e37f01 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -1,4 +1,4 @@
-.. doc_guidelines:
+.. _doc_guidelines:
DPDK Documentation Guidelines
=============================
diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index 561427b..f49ca88 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -9,3 +9,4 @@ Contributor's Guidelines
design
versioning
documentation
+ patches
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
new file mode 100644
index 0000000..7166014
--- /dev/null
+++ b/doc/guides/contributing/patches.rst
@@ -0,0 +1,327 @@
+.. submitting_patches:
+
+Contributing Code to DPDK
+=========================
+
+This document outlines the guidelines for submitting code to DPDK.
+
+The DPDK development process is modeled (loosely) on the Linux Kernel development model so it is worth reading the
+Linux kernel guide on submitting patches:
+`How to Get Your Change Into the Linux Kernel <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_.
+The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
+
+
+The DPDK Development Process
+-----------------------------
+
+The DPDK development process has the following features:
+
+* The code is hosted in a public git repository.
+* There is a mailing list where developers submit patches.
+* There are maintainers for hierarchical components.
+* Patches are reviewed publicly on the mailing list.
+* Successfully reviewed patches are merged to the master branch of the repository.
+
+The mailing list for DPDK development is `dev@dpkg.org <http://dpdk.org/ml/archives/dev/>`_.
+Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
+It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwxispork/project/dpdk/list/>`_
+
+The development process requires some familiarity with the ``git`` version control system.
+Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
+
+
+Getting the Source Code
+-----------------------
+
+The source code can be cloned using either of the following::
+
+ git clone git://dpdk.org/dpdk
+
+ git clone http://dpdk.org/git/dpdk
+
+
+Make your Changes
+-----------------
+
+Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
+
+* Follow the :ref:`coding_style` guidelines.
+
+* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
+
+* New external functions should be added to the local ``version.map`` file.
+ See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
+
+* Important changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
+ See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
+
+* Run ``make test`` and ``make examples`` to ensure the changes haven't broken existing code.
+
+* Don’t break compilation between commits with forward dependencies in a patchset.
+ Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
+
+* Add tests to the the ``app/test`` unit test framework where possible.
+
+* Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
+ See the :ref:`Documentation Guidelines <doc_guidelines>`.
+
+Once the changes have been made you should commit them to your local repo.
+
+For small changes, that do not require specific explanations, it is better to keep things together in the
+same patch.
+Larger changes that require different explanations should be separated into logical patches in a patchset.
+A good way of thinking about whether a patch should be split is to consider whether the change could be
+applied without dependencies as a backport.
+
+As a guide to how patches should be structured run ``git log`` on similar files.
+
+
+Commit Messages: Subject Line
+-----------------------------
+
+The first, summary, line of the git commit message becomes the subject line of the patch email.
+Here are some guidelines for the summary line:
+
+* The summary line must capture the area and the impact of the change.
+
+* The summary line should be around 50 characters.
+
+* The summary line should be lowercase apart from acronyms.
+
+* It should be prefixed with the component name (use git log to check existing components).
+ For example::
+
+ config: enable same drivers options for linux and bsd
+
+ ixgbe: fix offload config option name
+
+* Use the imperative of the verb (like instructions to the code base).
+ For example::
+
+ ixgbe: fix rss in 32 bit
+
+* Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
+
+The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
+for example: ``PATCH v2``.
+The is generally added by ``git send-email`` or ``git format-patch``, see below.
+
+If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
+An RFC patch doesn't have to be complete.
+It is intended as a way of getting early feedback.
+
+
+Commit Messages: Body
+---------------------
+
+Here are some guidelines for the body of a commit message:
+
+* The body of the message should describe the issue being fixed or the feature being added.
+ It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
+
+* When the change is obvious the body can be blank, apart from the signoff.
+
+* The commit message must end with a ``Signed-off-by:`` line which is added using::
+
+ git commit --signoff # or -s
+
+ The purpose of the signoff is explained in the
+ `Developer's Certificate of Origin <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_
+ section of the Linux kernel guidelines.
+
+ .. Note::
+
+ All developers must ensure that they have read and understood the
+ Developer's Certificate of Origin section of the documentation prior
+ to applying the signoff and submitting a patch.
+
+* The signoff must be a real name and not an alias or nickname.
+ More than one signoff is allowed.
+
+* The text of the commit message should be wrapped at 72 characters.
+
+* When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
+ You can generate the required text using the following git alias::
+
+ git alias: fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'
+
+
+ The ``Fixes:`` line can then be added to the commit message::
+
+ doc: fix vhost sample parameter
+
+ Update the docs to reflect removed dev-index.
+
+ Fixes: 17b8320a3e11 ("vhost: remove index parameter")
+
+ Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
+
+* When fixing an error or warning it is useful to add the error message and instructions on how to reproduce it.
+
+* 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.
+
+
+Creating Patches
+----------------
+
+It is possible to send patches directly from git but for new contributors it is recommended to generate the
+patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
+send them with ``git send-mail``.
+
+Here are some examples of using ``git format-patch`` to generate patches:
+
+.. code-block:: console
+
+ # Generate a patch from the last commit.
+ git format-patch -1
+
+ # Generate a patch from the last 3 commits.
+ git format-patch -3
+
+ # Generate the patches in a directory.
+ git format-patch -3 -o ~/patch/
+
+ # Add a cover letter to explain a patchset.
+ git format-patch -3 -o --cover-letter
+
+ # Add a prefix with a version number.
+ git format-patch -3 -o --subject-prefix 'PATCH v2'
+
+
+Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches.
+Smaller notes can be put inline in the patch after the ``---`` separator, for example::
+
+ Subject: [PATCH] fm10k/base: add FM10420 device ids
+
+ Add the device ID for Boulder Rapids and Atwood Channel to enable
+ drivers to support those devices.
+
+ Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
+ ---
+
+ ADD NOTES HERE.
+
+ drivers/net/fm10k/base/fm10k_api.c | 6 ++++++
+ drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
+ 2 files changed, 12 insertions(+)
+ ...
+
+Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
+This can be added to the cover letter or the annotations.
+For example::
+
+ v3:
+ * Fixed issued with version.map.
+
+ v2:
+ * Added i40e support.
+ * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
+ since 802.1AS can be supported through the same interfaces.
+
+
+Checking the Patches
+--------------------
+
+Patches should be checked for formatting and syntax issues using the Linux scripts tool ``checkpatch``.
+
+The ``checkpatch`` utility can be obtained by cloning, and periodically, updating the Linux kernel sources.
+
+The kernel guidelines tested by ``checkpatch`` don't match the DPDK Coding Style guidelines exactly but
+they provide a good indication of conformance.
+Warnings about kernel data types or about split strings can be ignored::
+
+ /path/checkpatch.pl --ignore PREFER_KERNEL_TYPES,SPLIT_STRING -q files*.patch
+
+Ensure that the code compiles with ``gcc`` and ``clang``::
+
+ make T=x86_64-native-linuxapp-gcc install
+ make T=x86_64-native-linuxapp-clang install
+
+Confirm that the changes haven't broken any existing code by using ``make test`` and ``make examples``.
+
+
+Sending Patches
+---------------
+
+Patches should be sent to the mailing list using ``git send-email``.
+You can configure an external SMTP with something like the following::
+
+ [sendemail]
+ smtpuser = name@domain.com
+ smtpserver = smtp.domain.com
+ smtpserverport = 465
+ smtpencryption = ssl
+
+See the `Git send-mail <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
+
+The patches should be sent to ``dev@dpdk.org``.
+If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``.
+The appropriate maintainer can be found in the ``MAINTAINERS`` file::
+
+ git send-email --to maintainer@some.org --cc dev@dpdk.org 000*.patch
+
+New additions can be sent without a maintainer::
+
+ git send-email --to dev@dpdk.org 000*.patch
+
+You can test the emails by sending it to yourself or with the ``--dry-run`` option.
+
+If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
+
+ git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
+
+The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
+`for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
+Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series.
+
+Once submitted your patches will appear on the mailing list and in Patchwork.
+
+Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step.
+The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending.
+
+
+The Review Process
+------------------
+
+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.
+
+#. Check the automatic test reports in the coming hours.
+
+#. Wait for review comments. While you are waiting review some other patches.
+
+#. Fix the review comments and submit a ``v n+1`` patchset::
+
+ git format-patch -3 -o --subject-prefix 'PATCH v2'
+
+#. Update Patchwork to mark your previous patches as "Superseded".
+
+#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
+ the patch with an email that includes something like::
+
+ Acked-by: Alex Smith <alex.smith@example.com>
+
+ **Note**: When acking patches please remove as much of the text of the patch email as possible.
+ It is generally best to delete everything after the ``Signed-off-by:`` line.
+
+#. Having the patch ``Reviewed-by:`` and/or ``Tested-by:`` will also help the patch to be accepted.
+
+#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
+ it may receive a ``nack``.
+ In this case you will need to make a more convincing technical argument in favor of your patches.
+
+#. In addition a patch will not be accepted if it doesn't address comments from a previous version with fixes or
+ valid arguments.
+
+#. Acked patches will be merged in the current or next merge window.
--
1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v3] doc: add contributors guide
2015-10-15 16:51 [dpdk-dev] [PATCH] add a dpdk contributors guide John McNamara
2015-10-15 16:51 ` [dpdk-dev] [PATCH] doc: add " John McNamara
2015-10-20 11:03 ` [dpdk-dev] [PATCH v2] " John McNamara
@ 2015-10-23 10:18 ` John McNamara
2015-11-18 13:38 ` Van Haaren, Harry
2015-11-20 12:46 ` [dpdk-dev] [PATCH v4] " John McNamara
3 siblings, 1 reply; 14+ messages in thread
From: John McNamara @ 2015-10-23 10:18 UTC (permalink / raw)
To: dev
Add a document to explain the DPDK patch submission and review process.
Signed-off-by: John McNamara <john.mcnamara@intel.com>
---
v3:
* Add recommendation to test build the shared and combined libraries.
v2:
* Fixes for mailing list comments.
* Fix for broken link target.
doc/guides/contributing/documentation.rst | 2 +-
doc/guides/contributing/index.rst | 1 +
doc/guides/contributing/patches.rst | 349 ++++++++++++++++++++++++++++++
3 files changed, 351 insertions(+), 1 deletion(-)
create mode 100644 doc/guides/contributing/patches.rst
diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
index 7c1eb41..0e37f01 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -1,4 +1,4 @@
-.. doc_guidelines:
+.. _doc_guidelines:
DPDK Documentation Guidelines
=============================
diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index 561427b..f49ca88 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -9,3 +9,4 @@ Contributor's Guidelines
design
versioning
documentation
+ patches
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
new file mode 100644
index 0000000..60bbdaa
--- /dev/null
+++ b/doc/guides/contributing/patches.rst
@@ -0,0 +1,349 @@
+.. submitting_patches:
+
+Contributing Code to DPDK
+=========================
+
+This document outlines the guidelines for submitting code to DPDK.
+
+The DPDK development process is modeled (loosely) on the Linux Kernel development model so it is worth reading the
+Linux kernel guide on submitting patches:
+`How to Get Your Change Into the Linux Kernel <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_.
+The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
+
+
+The DPDK Development Process
+-----------------------------
+
+The DPDK development process has the following features:
+
+* The code is hosted in a public git repository.
+* There is a mailing list where developers submit patches.
+* There are maintainers for hierarchical components.
+* Patches are reviewed publicly on the mailing list.
+* Successfully reviewed patches are merged to the master branch of the repository.
+
+The mailing list for DPDK development is `dev@dpkg.org <http://dpdk.org/ml/archives/dev/>`_.
+Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
+It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwxispork/project/dpdk/list/>`_
+
+The development process requires some familiarity with the ``git`` version control system.
+Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
+
+
+Getting the Source Code
+-----------------------
+
+The source code can be cloned using either of the following::
+
+ git clone git://dpdk.org/dpdk
+
+ git clone http://dpdk.org/git/dpdk
+
+
+Make your Changes
+-----------------
+
+Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
+
+* Follow the :ref:`coding_style` guidelines.
+
+* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
+
+* New external functions should be added to the local ``version.map`` file.
+ See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
+
+* Important changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
+ See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
+
+* Run ``make install``, ``make examples`` and ``make test`` and build the shared and combined libraries
+ to ensure the changes haven't broken existing code:
+
+ .. code-block:: console
+
+ export RTE_TARGET=x86_64-native-linuxapp-gcc
+
+ make T=$RTE_TARGET install
+ make T=$RTE_TARGET install CONFIG_RTE_BUILD_SHARED_LIB=y \
+ CONFIG_RTE_BUILD_COMBINE_LIBS=y
+ make T=$RTE_TARGET examples
+ make T=$RTE_TARGET test
+
+* Don’t break compilation between commits with forward dependencies in a patchset.
+ Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
+
+* Add tests to the the ``app/test`` unit test framework where possible.
+
+* Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
+ See the :ref:`Documentation Guidelines <doc_guidelines>`.
+
+Once the changes have been made you should commit them to your local repo.
+
+For small changes, that do not require specific explanations, it is better to keep things together in the
+same patch.
+Larger changes that require different explanations should be separated into logical patches in a patchset.
+A good way of thinking about whether a patch should be split is to consider whether the change could be
+applied without dependencies as a backport.
+
+As a guide to how patches should be structured run ``git log`` on similar files.
+
+
+Commit Messages: Subject Line
+-----------------------------
+
+The first, summary, line of the git commit message becomes the subject line of the patch email.
+Here are some guidelines for the summary line:
+
+* The summary line must capture the area and the impact of the change.
+
+* The summary line should be around 50 characters.
+
+* The summary line should be lowercase apart from acronyms.
+
+* It should be prefixed with the component name (use git log to check existing components).
+ For example::
+
+ config: enable same drivers options for linux and bsd
+
+ ixgbe: fix offload config option name
+
+* Use the imperative of the verb (like instructions to the code base).
+ For example::
+
+ ixgbe: fix rss in 32 bit
+
+* Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
+
+The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
+for example: ``PATCH v2``.
+The is generally added by ``git send-email`` or ``git format-patch``, see below.
+
+If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
+An RFC patch doesn't have to be complete.
+It is intended as a way of getting early feedback.
+
+
+Commit Messages: Body
+---------------------
+
+Here are some guidelines for the body of a commit message:
+
+* The body of the message should describe the issue being fixed or the feature being added.
+ It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
+
+* When the change is obvious the body can be blank, apart from the signoff.
+
+* The commit message must end with a ``Signed-off-by:`` line which is added using::
+
+ git commit --signoff # or -s
+
+ The purpose of the signoff is explained in the
+ `Developer's Certificate of Origin <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_
+ section of the Linux kernel guidelines.
+
+ .. Note::
+
+ All developers must ensure that they have read and understood the
+ Developer's Certificate of Origin section of the documentation prior
+ to applying the signoff and submitting a patch.
+
+* The signoff must be a real name and not an alias or nickname.
+ More than one signoff is allowed.
+
+* The text of the commit message should be wrapped at 72 characters.
+
+* When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
+ You can generate the required text using the following git alias::
+
+ git alias: fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'
+
+
+ The ``Fixes:`` line can then be added to the commit message::
+
+ doc: fix vhost sample parameter
+
+ Update the docs to reflect removed dev-index.
+
+ Fixes: 17b8320a3e11 ("vhost: remove index parameter")
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+
+* When fixing an error or warning it is useful to add the error message and instructions on how to reproduce it.
+
+* 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.
+
+
+Creating Patches
+----------------
+
+It is possible to send patches directly from git but for new contributors it is recommended to generate the
+patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
+send them with ``git send-mail``.
+
+Here are some examples of using ``git format-patch`` to generate patches:
+
+.. code-block:: console
+
+ # Generate a patch from the last commit.
+ git format-patch -1
+
+ # Generate a patch from the last 3 commits.
+ git format-patch -3
+
+ # Generate the patches in a directory.
+ git format-patch -3 -o ~/patch/
+
+ # Add a cover letter to explain a patchset.
+ git format-patch -3 -o --cover-letter
+
+ # Add a prefix with a version number.
+ git format-patch -3 -o --subject-prefix 'PATCH v2'
+
+
+Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches.
+Smaller notes can be put inline in the patch after the ``---`` separator, for example::
+
+ Subject: [PATCH] fm10k/base: add FM10420 device ids
+
+ Add the device ID for Boulder Rapids and Atwood Channel to enable
+ drivers to support those devices.
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+ ---
+
+ ADD NOTES HERE.
+
+ drivers/net/fm10k/base/fm10k_api.c | 6 ++++++
+ drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
+ 2 files changed, 12 insertions(+)
+ ...
+
+Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
+This can be added to the cover letter or the annotations.
+For example::
+
+ v3:
+ * Fixed issued with version.map.
+
+ v2:
+ * Added i40e support.
+ * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
+ since 802.1AS can be supported through the same interfaces.
+
+
+Checking the Patches
+--------------------
+
+Patches should be checked for formatting and syntax issues using the Linux scripts tool ``checkpatch``.
+
+The ``checkpatch`` utility can be obtained by cloning, and periodically, updating the Linux kernel sources.
+
+The kernel guidelines tested by ``checkpatch`` don't match the DPDK Coding Style guidelines exactly but
+they provide a good indication of conformance.
+Warnings about kernel data types or about split strings can be ignored::
+
+ /path/checkpatch.pl --ignore PREFER_KERNEL_TYPES,SPLIT_STRING -q files*.patch
+
+Ensure that the code compiles with ``gcc`` and ``clang``::
+
+ make T=x86_64-native-linuxapp-gcc install
+ make T=x86_64-native-linuxapp-clang install
+
+Confirm that the changes haven't broken any existing code by running ``make install``, ``make examples`` and
+``make test`` and building the shared and combined libraries:
+
+ .. code-block:: console
+
+ export RTE_TARGET=x86_64-native-linuxapp-gcc
+
+ make T=$RTE_TARGET install
+ make T=$RTE_TARGET install CONFIG_RTE_BUILD_SHARED_LIB=y \
+ CONFIG_RTE_BUILD_COMBINE_LIBS=y
+ make T=$RTE_TARGET examples
+ make T=$RTE_TARGET test
+
+
+Sending Patches
+---------------
+
+Patches should be sent to the mailing list using ``git send-email``.
+You can configure an external SMTP with something like the following::
+
+ [sendemail]
+ smtpuser = name@domain.com
+ smtpserver = smtp.domain.com
+ smtpserverport = 465
+ smtpencryption = ssl
+
+See the `Git send-mail <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
+
+The patches should be sent to ``dev@dpdk.org``.
+If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``.
+The appropriate maintainer can be found in the ``MAINTAINERS`` file::
+
+ git send-email --to maintainer@some.org --cc dev@dpdk.org 000*.patch
+
+New additions can be sent without a maintainer::
+
+ git send-email --to dev@dpdk.org 000*.patch
+
+You can test the emails by sending it to yourself or with the ``--dry-run`` option.
+
+If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
+
+ git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
+
+The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
+`for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
+Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series.
+
+Once submitted your patches will appear on the mailing list and in Patchwork.
+
+Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step.
+The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending.
+
+
+The Review Process
+------------------
+
+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.
+
+#. Check the automatic test reports in the coming hours.
+
+#. Wait for review comments. While you are waiting review some other patches.
+
+#. Fix the review comments and submit a ``v n+1`` patchset::
+
+ git format-patch -3 -o --subject-prefix 'PATCH v2'
+
+#. Update Patchwork to mark your previous patches as "Superseded".
+
+#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
+ the patch with an email that includes something like::
+
+ Acked-by: Alex Smith <alex.smith@example.com>
+
+ **Note**: When acking patches please remove as much of the text of the patch email as possible.
+ It is generally best to delete everything after the ``Signed-off-by:`` line.
+
+#. Having the patch ``Reviewed-by:`` and/or ``Tested-by:`` will also help the patch to be accepted.
+
+#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
+ it may receive a ``nack``.
+ In this case you will need to make a more convincing technical argument in favor of your patches.
+
+#. In addition a patch will not be accepted if it doesn't address comments from a previous version with fixes or
+ valid arguments.
+
+#. Acked patches will be merged in the current or next merge window.
--
1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v3] doc: add contributors guide
2015-10-23 10:18 ` [dpdk-dev] [PATCH v3] " John McNamara
@ 2015-11-18 13:38 ` Van Haaren, Harry
0 siblings, 0 replies; 14+ messages in thread
From: Van Haaren, Harry @ 2015-11-18 13:38 UTC (permalink / raw)
To: Mcnamara, John, dev
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of John McNamara
> Add a document to explain the DPDK patch submission and review process.
>
> Signed-off-by: John McNamara <john.mcnamara@intel.com>
> +* New external functions should be added to the local ``version.map`` file.
These should be listed alphabetically, perhaps add a note for that.
> + config: enable same drivers options for linux and bsd
> + ixgbe: fix rss in 32 bit
BSD and RSS should be uppercase
> +send them with ``git send-mail``.
<snip>
> +See the `Git send-mail
Typos, should be send-email
+ git alias: fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'
That line didn't work here - I used the following to set up the alias:
git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'"
Doxygen isn't mentioned, when creating a new header file, it should get a new page in the API docs.
After improvements listed above,
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v4] doc: add contributors guide
2015-10-15 16:51 [dpdk-dev] [PATCH] add a dpdk contributors guide John McNamara
` (2 preceding siblings ...)
2015-10-23 10:18 ` [dpdk-dev] [PATCH v3] " John McNamara
@ 2015-11-20 12:46 ` John McNamara
2015-12-13 22:35 ` Thomas Monjalon
` (2 more replies)
3 siblings, 3 replies; 14+ messages in thread
From: John McNamara @ 2015-11-20 12:46 UTC (permalink / raw)
To: dev
Add a document to explain the DPDK patch submission and review process.
Signed-off-by: John McNamara <john.mcnamara@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
---
v4:
* Fixes for mailing list comments.
v3:
* Add recommendation to test build the shared and combined libraries.
v2:
* Fixes for mailing list comments.
* Fix for broken link target.
doc/guides/contributing/documentation.rst | 2 +-
doc/guides/contributing/index.rst | 1 +
doc/guides/contributing/patches.rst | 350 ++++++++++++++++++++++++++++++
3 files changed, 352 insertions(+), 1 deletion(-)
create mode 100644 doc/guides/contributing/patches.rst
diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
index 7f5f061..c2d0461 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -1,4 +1,4 @@
-.. doc_guidelines:
+.. _doc_guidelines:
DPDK Documentation Guidelines
=============================
diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index 561427b..f49ca88 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -9,3 +9,4 @@ Contributor's Guidelines
design
versioning
documentation
+ patches
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
new file mode 100644
index 0000000..50992fd
--- /dev/null
+++ b/doc/guides/contributing/patches.rst
@@ -0,0 +1,350 @@
+.. submitting_patches:
+
+Contributing Code to DPDK
+=========================
+
+This document outlines the guidelines for submitting code to DPDK.
+
+The DPDK development process is modeled (loosely) on the Linux Kernel development model so it is worth reading the
+Linux kernel guide on submitting patches:
+`How to Get Your Change Into the Linux Kernel <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_.
+The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
+
+
+The DPDK Development Process
+-----------------------------
+
+The DPDK development process has the following features:
+
+* The code is hosted in a public git repository.
+* There is a mailing list where developers submit patches.
+* There are maintainers for hierarchical components.
+* Patches are reviewed publicly on the mailing list.
+* Successfully reviewed patches are merged to the master branch of the repository.
+
+The mailing list for DPDK development is `dev@dpkg.org <http://dpdk.org/ml/archives/dev/>`_.
+Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
+It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwxispork/project/dpdk/list/>`_
+
+The development process requires some familiarity with the ``git`` version control system.
+Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
+
+
+Getting the Source Code
+-----------------------
+
+The source code can be cloned using either of the following::
+
+ git clone git://dpdk.org/dpdk
+
+ git clone http://dpdk.org/git/dpdk
+
+
+Make your Changes
+-----------------
+
+Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
+
+* Follow the :ref:`coding_style` guidelines.
+
+* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
+
+* New external functions should be added to the local ``version.map`` file.
+ See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
+ New external functions should also be added in alphabetical order.
+
+* Important changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
+ See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
+
+* Run ``make install``, ``make examples`` and ``make test`` and build the shared and combined libraries
+ to ensure the changes haven't broken existing code:
+
+ .. code-block:: console
+
+ export RTE_TARGET=x86_64-native-linuxapp-gcc
+
+ make T=$RTE_TARGET install
+ make T=$RTE_TARGET install CONFIG_RTE_BUILD_SHARED_LIB=y \
+ CONFIG_RTE_BUILD_COMBINE_LIBS=y
+ make T=$RTE_TARGET examples
+ make T=$RTE_TARGET test
+
+* Don't break compilation between commits with forward dependencies in a patchset.
+ Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
+
+* Add tests to the the ``app/test`` unit test framework where possible.
+
+* Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
+ See the :ref:`Documentation Guidelines <doc_guidelines>`.
+
+Once the changes have been made you should commit them to your local repo.
+
+For small changes, that do not require specific explanations, it is better to keep things together in the
+same patch.
+Larger changes that require different explanations should be separated into logical patches in a patchset.
+A good way of thinking about whether a patch should be split is to consider whether the change could be
+applied without dependencies as a backport.
+
+As a guide to how patches should be structured run ``git log`` on similar files.
+
+
+Commit Messages: Subject Line
+-----------------------------
+
+The first, summary, line of the git commit message becomes the subject line of the patch email.
+Here are some guidelines for the summary line:
+
+* The summary line must capture the area and the impact of the change.
+
+* The summary line should be around 50 characters.
+
+* The summary line should be lowercase apart from acronyms.
+
+* It should be prefixed with the component name (use git log to check existing components).
+ For example::
+
+ ixgbe: fix offload config option name
+
+ config: increase max queues per port
+
+* Use the imperative of the verb (like instructions to the code base).
+ For example::
+
+ ixgbe: fix compilarion in 32 bit
+
+* Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
+
+The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
+for example: ``PATCH v2``.
+The is generally added by ``git send-email`` or ``git format-patch``, see below.
+
+If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
+An RFC patch doesn't have to be complete.
+It is intended as a way of getting early feedback.
+
+
+Commit Messages: Body
+---------------------
+
+Here are some guidelines for the body of a commit message:
+
+* The body of the message should describe the issue being fixed or the feature being added.
+ It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
+
+* When the change is obvious the body can be blank, apart from the signoff.
+
+* The commit message must end with a ``Signed-off-by:`` line which is added using::
+
+ git commit --signoff # or -s
+
+ The purpose of the signoff is explained in the
+ `Developer's Certificate of Origin <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_
+ section of the Linux kernel guidelines.
+
+ .. Note::
+
+ All developers must ensure that they have read and understood the
+ Developer's Certificate of Origin section of the documentation prior
+ to applying the signoff and submitting a patch.
+
+* The signoff must be a real name and not an alias or nickname.
+ More than one signoff is allowed.
+
+* The text of the commit message should be wrapped at 72 characters.
+
+* When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
+ You can generate the required text using the following git alias::
+
+ git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'"
+
+ The ``Fixes:`` line can then be added to the commit message::
+
+ doc: fix vhost sample parameter
+
+ Update the docs to reflect removed dev-index.
+
+ Fixes: 17b8320a3e11 ("vhost: remove index parameter")
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+
+* When fixing an error or warning it is useful to add the error message and instructions on how to reproduce it.
+
+* 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.
+
+
+Creating Patches
+----------------
+
+It is possible to send patches directly from git but for new contributors it is recommended to generate the
+patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
+send them with ``git send-email``.
+
+Here are some examples of using ``git format-patch`` to generate patches:
+
+.. code-block:: console
+
+ # Generate a patch from the last commit.
+ git format-patch -1
+
+ # Generate a patch from the last 3 commits.
+ git format-patch -3
+
+ # Generate the patches in a directory.
+ git format-patch -3 -o ~/patch/
+
+ # Add a cover letter to explain a patchset.
+ git format-patch -3 -o ~/patch/ --cover-letter
+
+ # Add a prefix with a version number.
+ git format-patch -3 -o ~/patch/ -v 2
+
+
+
+Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches.
+Smaller notes can be put inline in the patch after the ``---`` separator, for example::
+
+ Subject: [PATCH] fm10k/base: add FM10420 device ids
+
+ Add the device ID for Boulder Rapids and Atwood Channel to enable
+ drivers to support those devices.
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+ ---
+
+ ADD NOTES HERE.
+
+ drivers/net/fm10k/base/fm10k_api.c | 6 ++++++
+ drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
+ 2 files changed, 12 insertions(+)
+ ...
+
+Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
+This can be added to the cover letter or the annotations.
+For example::
+
+ v3:
+ * Fixed issued with version.map.
+
+ v2:
+ * Added i40e support.
+ * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
+ since 802.1AS can be supported through the same interfaces.
+
+
+Checking the Patches
+--------------------
+
+Patches should be checked for formatting and syntax issues using the Linux scripts tool ``checkpatch``.
+
+The ``checkpatch`` utility can be obtained by cloning, and periodically, updating the Linux kernel sources.
+
+The kernel guidelines tested by ``checkpatch`` don't match the DPDK Coding Style guidelines exactly but
+they provide a good indication of conformance.
+Warnings about kernel data types or about split strings can be ignored::
+
+ /path/checkpatch.pl --ignore PREFER_KERNEL_TYPES,SPLIT_STRING -q files*.patch
+
+Ensure that the code compiles with ``gcc`` and ``clang``::
+
+ make T=x86_64-native-linuxapp-gcc install
+ make T=x86_64-native-linuxapp-clang install
+
+Confirm that the changes haven't broken any existing code by running ``make install``, ``make examples`` and
+``make test`` and building the shared and combined libraries:
+
+ .. code-block:: console
+
+ export RTE_TARGET=x86_64-native-linuxapp-gcc
+
+ make T=$RTE_TARGET install
+ make T=$RTE_TARGET install CONFIG_RTE_BUILD_SHARED_LIB=y \
+ CONFIG_RTE_BUILD_COMBINE_LIBS=y
+ make T=$RTE_TARGET examples
+ make T=$RTE_TARGET test
+
+
+Sending Patches
+---------------
+
+Patches should be sent to the mailing list using ``git send-email``.
+You can configure an external SMTP with something like the following::
+
+ [sendemail]
+ smtpuser = name@domain.com
+ smtpserver = smtp.domain.com
+ smtpserverport = 465
+ smtpencryption = ssl
+
+See the `Git send-email <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
+
+The patches should be sent to ``dev@dpdk.org``.
+If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``.
+The appropriate maintainer can be found in the ``MAINTAINERS`` file::
+
+ git send-email --to maintainer@some.org --cc dev@dpdk.org 000*.patch
+
+New additions can be sent without a maintainer::
+
+ git send-email --to dev@dpdk.org 000*.patch
+
+You can test the emails by sending it to yourself or with the ``--dry-run`` option.
+
+If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
+
+ git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
+
+The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
+`for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
+Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series.
+
+Once submitted your patches will appear on the mailing list and in Patchwork.
+
+Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step.
+The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending.
+
+
+The Review Process
+------------------
+
+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.
+
+#. Check the automatic test reports in the coming hours.
+
+#. Wait for review comments. While you are waiting review some other patches.
+
+#. Fix the review comments and submit a ``v n+1`` patchset::
+
+ git format-patch -3 -v 2
+
+#. Update Patchwork to mark your previous patches as "Superseded".
+
+#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
+ the patch with an email that includes something like::
+
+ Acked-by: Alex Smith <alex.smith@example.com>
+
+ **Note**: When acking patches please remove as much of the text of the patch email as possible.
+ It is generally best to delete everything after the ``Signed-off-by:`` line.
+
+#. Having the patch ``Reviewed-by:`` and/or ``Tested-by:`` will also help the patch to be accepted.
+
+#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
+ it may receive a ``nack``.
+ In this case you will need to make a more convincing technical argument in favor of your patches.
+
+#. In addition a patch will not be accepted if it doesn't address comments from a previous version with fixes or
+ valid arguments.
+
+#. Acked patches will be merged in the current or next merge window.
--
2.5.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v4] doc: add contributors guide
2015-11-20 12:46 ` [dpdk-dev] [PATCH v4] " John McNamara
@ 2015-12-13 22:35 ` Thomas Monjalon
2015-12-14 9:31 ` Mcnamara, John
2015-12-14 20:32 ` [dpdk-dev] [PATCH v2] " John McNamara
2015-12-14 20:45 ` [dpdk-dev] [PATCH v5] " John McNamara
2 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2015-12-13 22:35 UTC (permalink / raw)
To: John McNamara; +Cc: dev
Thanks for this very useful doc, John.
2015-11-20 12:46, John McNamara:
> +* Run ``make install``, ``make examples`` and ``make test`` and build the shared and combined libraries
> + to ensure the changes haven't broken existing code:
> +
> + .. code-block:: console
> +
> + export RTE_TARGET=x86_64-native-linuxapp-gcc
> +
> + make T=$RTE_TARGET install
> + make T=$RTE_TARGET install CONFIG_RTE_BUILD_SHARED_LIB=y \
> + CONFIG_RTE_BUILD_COMBINE_LIBS=y
> + make T=$RTE_TARGET examples
> + make T=$RTE_TARGET test
These commands are superseded by a new syntax and may be replaced by the use
of the script test-build.sh.
> +* Use the imperative of the verb (like instructions to the code base).
> + For example::
> +
> + ixgbe: fix compilarion in 32 bit
Typo.
This example is not needed (there is one above).
> +The kernel guidelines tested by ``checkpatch`` don't match the DPDK Coding Style guidelines exactly but
> +they provide a good indication of conformance.
> +Warnings about kernel data types or about split strings can be ignored::
> +
> + /path/checkpatch.pl --ignore PREFER_KERNEL_TYPES,SPLIT_STRING -q files*.patch
The script checkpatches.sh should be used. It includes a list of options to ignore.
> +Ensure that the code compiles with ``gcc`` and ``clang``::
> +
> + make T=x86_64-native-linuxapp-gcc install
> + make T=x86_64-native-linuxapp-clang install
It may be done with
scripts/test-build.sh x86_64-native-linuxapp-gcc x86_64-native-linuxapp-clang
My preferred configs are:
x86_64-native-linuxapp-gcc+shared+next
x86_64-native-linuxapp-clang+shared+combined
i686-native-linuxapp-gcc+combined
And these options are common:
export DPDK_DEP_ZLIB=y
export DPDK_DEP_PCAP=y
export DPDK_DEP_SSL=y
> +Confirm that the changes haven't broken any existing code by running ``make install``, ``make examples`` and
> +``make test`` and building the shared and combined libraries:
> +
> + .. code-block:: console
> +
> + export RTE_TARGET=x86_64-native-linuxapp-gcc
> +
> + make T=$RTE_TARGET install
> + make T=$RTE_TARGET install CONFIG_RTE_BUILD_SHARED_LIB=y \
> + CONFIG_RTE_BUILD_COMBINE_LIBS=y
> + make T=$RTE_TARGET examples
> + make T=$RTE_TARGET test
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v4] doc: add contributors guide
2015-12-13 22:35 ` Thomas Monjalon
@ 2015-12-14 9:31 ` Mcnamara, John
0 siblings, 0 replies; 14+ messages in thread
From: Mcnamara, John @ 2015-12-14 9:31 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Sunday, December 13, 2015 10:35 PM
> To: Mcnamara, John
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] doc: add contributors guide
>
> Thanks for this very useful doc, John.
>
> 2015-11-20 12:46, John McNamara:
> > +* Run ``make install``, ``make examples`` and ``make test`` and build
> > +the shared and combined libraries
> > + to ensure the changes haven't broken existing code:
> > +
> > + .. code-block:: console
> > +
> > + export RTE_TARGET=x86_64-native-linuxapp-gcc
> > +
> > + make T=$RTE_TARGET install
> > + make T=$RTE_TARGET install CONFIG_RTE_BUILD_SHARED_LIB=y \
> > + CONFIG_RTE_BUILD_COMBINE_LIBS=y
> > + make T=$RTE_TARGET examples
> > + make T=$RTE_TARGET test
>
> These commands are superseded by a new syntax and may be replaced by the
> use of the script test-build.sh.
Hi,
I'll make these, and the other changes. I'll put the checkpatch and build testing in their own sections since they are referred to from more than one place and it will probably be useful to link to them directly.
John.
--
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2] doc: add contributors guide
2015-11-20 12:46 ` [dpdk-dev] [PATCH v4] " John McNamara
2015-12-13 22:35 ` Thomas Monjalon
@ 2015-12-14 20:32 ` John McNamara
2015-12-14 20:46 ` Mcnamara, John
2015-12-14 20:45 ` [dpdk-dev] [PATCH v5] " John McNamara
2 siblings, 1 reply; 14+ messages in thread
From: John McNamara @ 2015-12-14 20:32 UTC (permalink / raw)
To: dev
Add a document to explain the DPDK patch submission and review process.
Signed-off-by: John McNamara <john.mcnamara@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
---
v2:
* Added references to new checkpatches.sh and test-build.sh scripts
as per Thomas' suggestion on the mailing list.
doc/guides/contributing/documentation.rst | 2 +-
doc/guides/contributing/index.rst | 1 +
doc/guides/contributing/patches.rst | 389 ++++++++++++++++++++++++++++++
3 files changed, 391 insertions(+), 1 deletion(-)
create mode 100644 doc/guides/contributing/patches.rst
diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
index 6dfaaa8..ba5c4de 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -1,4 +1,4 @@
-.. doc_guidelines:
+.. _doc_guidelines:
DPDK Documentation Guidelines
=============================
diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index 561427b..f49ca88 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -9,3 +9,4 @@ Contributor's Guidelines
design
versioning
documentation
+ patches
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
new file mode 100644
index 0000000..1729c6b
--- /dev/null
+++ b/doc/guides/contributing/patches.rst
@@ -0,0 +1,389 @@
+.. submitting_patches:
+
+Contributing Code to DPDK
+=========================
+
+This document outlines the guidelines for submitting code to DPDK.
+
+The DPDK development process is modeled (loosely) on the Linux Kernel development model so it is worth reading the
+Linux kernel guide on submitting patches:
+`How to Get Your Change Into the Linux Kernel <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_.
+The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
+
+
+The DPDK Development Process
+-----------------------------
+
+The DPDK development process has the following features:
+
+* The code is hosted in a public git repository.
+* There is a mailing list where developers submit patches.
+* There are maintainers for hierarchical components.
+* Patches are reviewed publicly on the mailing list.
+* Successfully reviewed patches are merged to the master branch of the repository.
+
+The mailing list for DPDK development is `dev@dpkg.org <http://dpdk.org/ml/archives/dev/>`_.
+Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
+It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwxispork/project/dpdk/list/>`_
+
+The development process requires some familiarity with the ``git`` version control system.
+Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
+
+
+Getting the Source Code
+-----------------------
+
+The source code can be cloned using either of the following::
+
+ git clone git://dpdk.org/dpdk
+
+ git clone http://dpdk.org/git/dpdk
+
+
+Make your Changes
+-----------------
+
+Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
+
+* Follow the :ref:`coding_style` guidelines.
+
+* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
+
+* New external functions should be added to the local ``version.map`` file.
+ See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
+ New external functions should also be added in alphabetical order.
+
+* Important changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
+ See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
+
+* Test the compilation works with different targets, compilers and options, see :ref:`contrib_check_compilation`.
+
+* Don't break compilation between commits with forward dependencies in a patchset.
+ Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
+
+* Add tests to the the ``app/test`` unit test framework where possible.
+
+* Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
+ See the :ref:`Documentation Guidelines <doc_guidelines>`.
+
+Once the changes have been made you should commit them to your local repo.
+
+For small changes, that do not require specific explanations, it is better to keep things together in the
+same patch.
+Larger changes that require different explanations should be separated into logical patches in a patchset.
+A good way of thinking about whether a patch should be split is to consider whether the change could be
+applied without dependencies as a backport.
+
+As a guide to how patches should be structured run ``git log`` on similar files.
+
+
+Commit Messages: Subject Line
+-----------------------------
+
+The first, summary, line of the git commit message becomes the subject line of the patch email.
+Here are some guidelines for the summary line:
+
+* The summary line must capture the area and the impact of the change.
+
+* The summary line should be around 50 characters.
+
+* The summary line should be lowercase apart from acronyms.
+
+* It should be prefixed with the component name (use git log to check existing components).
+ For example::
+
+ ixgbe: fix offload config option name
+
+ config: increase max queues per port
+
+* Use the imperative of the verb (like instructions to the code base).
+
+* Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
+
+The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
+for example: ``PATCH v2``.
+The is generally added by ``git send-email`` or ``git format-patch``, see below.
+
+If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
+An RFC patch doesn't have to be complete.
+It is intended as a way of getting early feedback.
+
+
+Commit Messages: Body
+---------------------
+
+Here are some guidelines for the body of a commit message:
+
+* The body of the message should describe the issue being fixed or the feature being added.
+ It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
+
+* When the change is obvious the body can be blank, apart from the signoff.
+
+* The commit message must end with a ``Signed-off-by:`` line which is added using::
+
+ git commit --signoff # or -s
+
+ The purpose of the signoff is explained in the
+ `Developer's Certificate of Origin <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_
+ section of the Linux kernel guidelines.
+
+ .. Note::
+
+ All developers must ensure that they have read and understood the
+ Developer's Certificate of Origin section of the documentation prior
+ to applying the signoff and submitting a patch.
+
+* The signoff must be a real name and not an alias or nickname.
+ More than one signoff is allowed.
+
+* The text of the commit message should be wrapped at 72 characters.
+
+* When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
+ You can generate the required text using the following git alias::
+
+ git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'"
+
+ The ``Fixes:`` line can then be added to the commit message::
+
+ doc: fix vhost sample parameter
+
+ Update the docs to reflect removed dev-index.
+
+ Fixes: 17b8320a3e11 ("vhost: remove index parameter")
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+
+* When fixing an error or warning it is useful to add the error message and instructions on how to reproduce it.
+
+* 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.
+
+
+Creating Patches
+----------------
+
+It is possible to send patches directly from git but for new contributors it is recommended to generate the
+patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
+send them with ``git send-email``.
+
+Here are some examples of using ``git format-patch`` to generate patches:
+
+.. code-block:: console
+
+ # Generate a patch from the last commit.
+ git format-patch -1
+
+ # Generate a patch from the last 3 commits.
+ git format-patch -3
+
+ # Generate the patches in a directory.
+ git format-patch -3 -o ~/patch/
+
+ # Add a cover letter to explain a patchset.
+ git format-patch -3 -o ~/patch/ --cover-letter
+
+ # Add a prefix with a version number.
+ git format-patch -3 -o ~/patch/ -v 2
+
+
+Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches.
+Smaller notes can be put inline in the patch after the ``---`` separator, for example::
+
+ Subject: [PATCH] fm10k/base: add FM10420 device ids
+
+ Add the device ID for Boulder Rapids and Atwood Channel to enable
+ drivers to support those devices.
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+ ---
+
+ ADD NOTES HERE.
+
+ drivers/net/fm10k/base/fm10k_api.c | 6 ++++++
+ drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
+ 2 files changed, 12 insertions(+)
+ ...
+
+Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
+This can be added to the cover letter or the annotations.
+For example::
+
+ ---
+ v3:
+ * Fixed issued with version.map.
+
+ v2:
+ * Added i40e support.
+ * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
+ since 802.1AS can be supported through the same interfaces.
+
+
+.. _contrib_checkpatch:
+
+Checking the Patches
+--------------------
+
+Patches should be checked for formatting and syntax issues using the ``checkpatches.sh`` script in the ``scripts``
+directory of the DPDK repo.
+This uses the Linux kernel development tool ``checkpatch.pl`` which can be obtained by cloning, and periodically,
+updating the Linux kernel sources.
+
+The path to the original Linux script must be set in the environment variable ``DPDK_CHECKPATCH_PATH``.
+This, and any other configuration variables required by the development tools, are loaded from the following
+files, in order of preference::
+
+ .develconfig
+ ~/.config/dpdk/devel.config
+ /etc/dpdk/devel.config.
+
+Once the environment variable the script can be run as follows::
+
+ scripts/checkpatches.sh ~/patch/
+
+The script usage is::
+
+ checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
+
+Where:
+
+* ``-h``: help, usage.
+* ``-q``: quiet. Don't output anything for files without issues.
+* ``-v``: verbose.
+* ``patchX``: path to one or more patches.
+
+
+.. _contrib_check_compilation:
+
+Checking Compilation
+--------------------
+
+Compilation of patches and changes should be tested using the the ``test-build.sh`` script in the ``scripts``
+directory of the DPDK repo::
+
+ scripts/test-build.sh x86_64-native-linuxapp-gcc+next+shared+combined
+
+The script usage is::
+
+ test-build.sh [-h] [-jX] [-s] [config1 [config2] ...]]
+
+Where:
+
+* ``-h``: help, usage.
+* ``-jX``: use X parallel jobs in "make".
+* ``-s``: short test with only first config and without examples/doc.
+* ``config``: default config name plus config switches delimited with a ``+`` sign.
+
+Examples of configs are::
+
+ x86_64-native-linuxapp-gcc
+ x86_64-native-linuxapp-gcc+next+shared+combined
+ x86_64-native-linuxapp-gcc+shared+next
+ x86_64-native-linuxapp-clang+shared+combined
+ i686-native-linuxapp-gcc+combined
+
+The builds can be modifies via the following environmental variables:
+
+* ``DPDK_BUILD_TEST_CONFIGS`` (target1+option1+option2 target2)
+* ``DPDK_DEP_CFLAGS``
+* ``DPDK_DEP_LDFLAGS``
+* ``DPDK_DEP_MOFED`` (y/[n])
+* ``DPDK_DEP_PCAP`` (y/[n])
+* ``DPDK_NOTIFY`` (notify-send)
+
+These can be set from the command line or in the config files shown above in the :ref:`contrib_checkpatch`.
+
+The recommended configurations and options to test compilation prior to submitting patches are::
+
+ x86_64-native-linuxapp-gcc+shared+next
+ x86_64-native-linuxapp-clang+shared+combined
+ i686-native-linuxapp-gcc+combined
+
+ export DPDK_DEP_ZLIB=y
+ export DPDK_DEP_PCAP=y
+ export DPDK_DEP_SSL=y
+
+
+Sending Patches
+---------------
+
+Patches should be sent to the mailing list using ``git send-email``.
+You can configure an external SMTP with something like the following::
+
+ [sendemail]
+ smtpuser = name@domain.com
+ smtpserver = smtp.domain.com
+ smtpserverport = 465
+ smtpencryption = ssl
+
+See the `Git send-email <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
+
+The patches should be sent to ``dev@dpdk.org``.
+If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``.
+The appropriate maintainer can be found in the ``MAINTAINERS`` file::
+
+ git send-email --to maintainer@some.org --cc dev@dpdk.org 000*.patch
+
+New additions can be sent without a maintainer::
+
+ git send-email --to dev@dpdk.org 000*.patch
+
+You can test the emails by sending it to yourself or with the ``--dry-run`` option.
+
+If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
+
+ git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
+
+The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
+`for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
+Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series.
+
+Once submitted your patches will appear on the mailing list and in Patchwork.
+
+Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step.
+The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending.
+
+
+The Review Process
+------------------
+
+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.
+
+#. Check the automatic test reports in the coming hours.
+
+#. Wait for review comments. While you are waiting review some other patches.
+
+#. Fix the review comments and submit a ``v n+1`` patchset::
+
+ git format-patch -3 -v 2
+
+#. Update Patchwork to mark your previous patches as "Superseded".
+
+#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
+ the patch with an email that includes something like::
+
+ Acked-by: Alex Smith <alex.smith@example.com>
+
+ **Note**: When acking patches please remove as much of the text of the patch email as possible.
+ It is generally best to delete everything after the ``Signed-off-by:`` line.
+
+#. Having the patch ``Reviewed-by:`` and/or ``Tested-by:`` will also help the patch to be accepted.
+
+#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
+ it may receive a ``nack``.
+ In this case you will need to make a more convincing technical argument in favor of your patches.
+
+#. In addition a patch will not be accepted if it doesn't address comments from a previous version with fixes or
+ valid arguments.
+
+#. Acked patches will be merged in the current or next merge window.
--
2.5.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v5] doc: add contributors guide
2015-11-20 12:46 ` [dpdk-dev] [PATCH v4] " John McNamara
2015-12-13 22:35 ` Thomas Monjalon
2015-12-14 20:32 ` [dpdk-dev] [PATCH v2] " John McNamara
@ 2015-12-14 20:45 ` John McNamara
2015-12-14 22:15 ` Thomas Monjalon
2 siblings, 1 reply; 14+ messages in thread
From: John McNamara @ 2015-12-14 20:45 UTC (permalink / raw)
To: dev
Add a document to explain the DPDK patch submission and review process.
Signed-off-by: John McNamara <john.mcnamara@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
---
v5:
* Added references to new checkpatches.sh and test-build.sh scripts
as per Thomas' suggestion on the mailing list.
v4:
* Fixes for mailing list comments.
v3:
* Add recommendation to test build the shared and combined libraries.
v2:
* Fixes for mailing list comments.
* Fix for broken link target.
doc/guides/contributing/documentation.rst | 2 +-
doc/guides/contributing/index.rst | 1 +
doc/guides/contributing/patches.rst | 389 ++++++++++++++++++++++++++++++
3 files changed, 391 insertions(+), 1 deletion(-)
create mode 100644 doc/guides/contributing/patches.rst
diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
index 6dfaaa8..ba5c4de 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -1,4 +1,4 @@
-.. doc_guidelines:
+.. _doc_guidelines:
DPDK Documentation Guidelines
=============================
diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index 561427b..f49ca88 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -9,3 +9,4 @@ Contributor's Guidelines
design
versioning
documentation
+ patches
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
new file mode 100644
index 0000000..1729c6b
--- /dev/null
+++ b/doc/guides/contributing/patches.rst
@@ -0,0 +1,389 @@
+.. submitting_patches:
+
+Contributing Code to DPDK
+=========================
+
+This document outlines the guidelines for submitting code to DPDK.
+
+The DPDK development process is modeled (loosely) on the Linux Kernel development model so it is worth reading the
+Linux kernel guide on submitting patches:
+`How to Get Your Change Into the Linux Kernel <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_.
+The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
+
+
+The DPDK Development Process
+-----------------------------
+
+The DPDK development process has the following features:
+
+* The code is hosted in a public git repository.
+* There is a mailing list where developers submit patches.
+* There are maintainers for hierarchical components.
+* Patches are reviewed publicly on the mailing list.
+* Successfully reviewed patches are merged to the master branch of the repository.
+
+The mailing list for DPDK development is `dev@dpkg.org <http://dpdk.org/ml/archives/dev/>`_.
+Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
+It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwxispork/project/dpdk/list/>`_
+
+The development process requires some familiarity with the ``git`` version control system.
+Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
+
+
+Getting the Source Code
+-----------------------
+
+The source code can be cloned using either of the following::
+
+ git clone git://dpdk.org/dpdk
+
+ git clone http://dpdk.org/git/dpdk
+
+
+Make your Changes
+-----------------
+
+Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
+
+* Follow the :ref:`coding_style` guidelines.
+
+* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
+
+* New external functions should be added to the local ``version.map`` file.
+ See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
+ New external functions should also be added in alphabetical order.
+
+* Important changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
+ See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
+
+* Test the compilation works with different targets, compilers and options, see :ref:`contrib_check_compilation`.
+
+* Don't break compilation between commits with forward dependencies in a patchset.
+ Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
+
+* Add tests to the the ``app/test`` unit test framework where possible.
+
+* Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
+ See the :ref:`Documentation Guidelines <doc_guidelines>`.
+
+Once the changes have been made you should commit them to your local repo.
+
+For small changes, that do not require specific explanations, it is better to keep things together in the
+same patch.
+Larger changes that require different explanations should be separated into logical patches in a patchset.
+A good way of thinking about whether a patch should be split is to consider whether the change could be
+applied without dependencies as a backport.
+
+As a guide to how patches should be structured run ``git log`` on similar files.
+
+
+Commit Messages: Subject Line
+-----------------------------
+
+The first, summary, line of the git commit message becomes the subject line of the patch email.
+Here are some guidelines for the summary line:
+
+* The summary line must capture the area and the impact of the change.
+
+* The summary line should be around 50 characters.
+
+* The summary line should be lowercase apart from acronyms.
+
+* It should be prefixed with the component name (use git log to check existing components).
+ For example::
+
+ ixgbe: fix offload config option name
+
+ config: increase max queues per port
+
+* Use the imperative of the verb (like instructions to the code base).
+
+* Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
+
+The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
+for example: ``PATCH v2``.
+The is generally added by ``git send-email`` or ``git format-patch``, see below.
+
+If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
+An RFC patch doesn't have to be complete.
+It is intended as a way of getting early feedback.
+
+
+Commit Messages: Body
+---------------------
+
+Here are some guidelines for the body of a commit message:
+
+* The body of the message should describe the issue being fixed or the feature being added.
+ It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
+
+* When the change is obvious the body can be blank, apart from the signoff.
+
+* The commit message must end with a ``Signed-off-by:`` line which is added using::
+
+ git commit --signoff # or -s
+
+ The purpose of the signoff is explained in the
+ `Developer's Certificate of Origin <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_
+ section of the Linux kernel guidelines.
+
+ .. Note::
+
+ All developers must ensure that they have read and understood the
+ Developer's Certificate of Origin section of the documentation prior
+ to applying the signoff and submitting a patch.
+
+* The signoff must be a real name and not an alias or nickname.
+ More than one signoff is allowed.
+
+* The text of the commit message should be wrapped at 72 characters.
+
+* When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
+ You can generate the required text using the following git alias::
+
+ git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'"
+
+ The ``Fixes:`` line can then be added to the commit message::
+
+ doc: fix vhost sample parameter
+
+ Update the docs to reflect removed dev-index.
+
+ Fixes: 17b8320a3e11 ("vhost: remove index parameter")
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+
+* When fixing an error or warning it is useful to add the error message and instructions on how to reproduce it.
+
+* 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.
+
+
+Creating Patches
+----------------
+
+It is possible to send patches directly from git but for new contributors it is recommended to generate the
+patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
+send them with ``git send-email``.
+
+Here are some examples of using ``git format-patch`` to generate patches:
+
+.. code-block:: console
+
+ # Generate a patch from the last commit.
+ git format-patch -1
+
+ # Generate a patch from the last 3 commits.
+ git format-patch -3
+
+ # Generate the patches in a directory.
+ git format-patch -3 -o ~/patch/
+
+ # Add a cover letter to explain a patchset.
+ git format-patch -3 -o ~/patch/ --cover-letter
+
+ # Add a prefix with a version number.
+ git format-patch -3 -o ~/patch/ -v 2
+
+
+Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches.
+Smaller notes can be put inline in the patch after the ``---`` separator, for example::
+
+ Subject: [PATCH] fm10k/base: add FM10420 device ids
+
+ Add the device ID for Boulder Rapids and Atwood Channel to enable
+ drivers to support those devices.
+
+ Signed-off-by: Alex Smith <alex.smith@example.com>
+ ---
+
+ ADD NOTES HERE.
+
+ drivers/net/fm10k/base/fm10k_api.c | 6 ++++++
+ drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
+ 2 files changed, 12 insertions(+)
+ ...
+
+Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
+This can be added to the cover letter or the annotations.
+For example::
+
+ ---
+ v3:
+ * Fixed issued with version.map.
+
+ v2:
+ * Added i40e support.
+ * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
+ since 802.1AS can be supported through the same interfaces.
+
+
+.. _contrib_checkpatch:
+
+Checking the Patches
+--------------------
+
+Patches should be checked for formatting and syntax issues using the ``checkpatches.sh`` script in the ``scripts``
+directory of the DPDK repo.
+This uses the Linux kernel development tool ``checkpatch.pl`` which can be obtained by cloning, and periodically,
+updating the Linux kernel sources.
+
+The path to the original Linux script must be set in the environment variable ``DPDK_CHECKPATCH_PATH``.
+This, and any other configuration variables required by the development tools, are loaded from the following
+files, in order of preference::
+
+ .develconfig
+ ~/.config/dpdk/devel.config
+ /etc/dpdk/devel.config.
+
+Once the environment variable the script can be run as follows::
+
+ scripts/checkpatches.sh ~/patch/
+
+The script usage is::
+
+ checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
+
+Where:
+
+* ``-h``: help, usage.
+* ``-q``: quiet. Don't output anything for files without issues.
+* ``-v``: verbose.
+* ``patchX``: path to one or more patches.
+
+
+.. _contrib_check_compilation:
+
+Checking Compilation
+--------------------
+
+Compilation of patches and changes should be tested using the the ``test-build.sh`` script in the ``scripts``
+directory of the DPDK repo::
+
+ scripts/test-build.sh x86_64-native-linuxapp-gcc+next+shared+combined
+
+The script usage is::
+
+ test-build.sh [-h] [-jX] [-s] [config1 [config2] ...]]
+
+Where:
+
+* ``-h``: help, usage.
+* ``-jX``: use X parallel jobs in "make".
+* ``-s``: short test with only first config and without examples/doc.
+* ``config``: default config name plus config switches delimited with a ``+`` sign.
+
+Examples of configs are::
+
+ x86_64-native-linuxapp-gcc
+ x86_64-native-linuxapp-gcc+next+shared+combined
+ x86_64-native-linuxapp-gcc+shared+next
+ x86_64-native-linuxapp-clang+shared+combined
+ i686-native-linuxapp-gcc+combined
+
+The builds can be modifies via the following environmental variables:
+
+* ``DPDK_BUILD_TEST_CONFIGS`` (target1+option1+option2 target2)
+* ``DPDK_DEP_CFLAGS``
+* ``DPDK_DEP_LDFLAGS``
+* ``DPDK_DEP_MOFED`` (y/[n])
+* ``DPDK_DEP_PCAP`` (y/[n])
+* ``DPDK_NOTIFY`` (notify-send)
+
+These can be set from the command line or in the config files shown above in the :ref:`contrib_checkpatch`.
+
+The recommended configurations and options to test compilation prior to submitting patches are::
+
+ x86_64-native-linuxapp-gcc+shared+next
+ x86_64-native-linuxapp-clang+shared+combined
+ i686-native-linuxapp-gcc+combined
+
+ export DPDK_DEP_ZLIB=y
+ export DPDK_DEP_PCAP=y
+ export DPDK_DEP_SSL=y
+
+
+Sending Patches
+---------------
+
+Patches should be sent to the mailing list using ``git send-email``.
+You can configure an external SMTP with something like the following::
+
+ [sendemail]
+ smtpuser = name@domain.com
+ smtpserver = smtp.domain.com
+ smtpserverport = 465
+ smtpencryption = ssl
+
+See the `Git send-email <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
+
+The patches should be sent to ``dev@dpdk.org``.
+If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``.
+The appropriate maintainer can be found in the ``MAINTAINERS`` file::
+
+ git send-email --to maintainer@some.org --cc dev@dpdk.org 000*.patch
+
+New additions can be sent without a maintainer::
+
+ git send-email --to dev@dpdk.org 000*.patch
+
+You can test the emails by sending it to yourself or with the ``--dry-run`` option.
+
+If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
+
+ git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
+
+The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
+`for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
+Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series.
+
+Once submitted your patches will appear on the mailing list and in Patchwork.
+
+Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step.
+The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending.
+
+
+The Review Process
+------------------
+
+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.
+
+#. Check the automatic test reports in the coming hours.
+
+#. Wait for review comments. While you are waiting review some other patches.
+
+#. Fix the review comments and submit a ``v n+1`` patchset::
+
+ git format-patch -3 -v 2
+
+#. Update Patchwork to mark your previous patches as "Superseded".
+
+#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
+ the patch with an email that includes something like::
+
+ Acked-by: Alex Smith <alex.smith@example.com>
+
+ **Note**: When acking patches please remove as much of the text of the patch email as possible.
+ It is generally best to delete everything after the ``Signed-off-by:`` line.
+
+#. Having the patch ``Reviewed-by:`` and/or ``Tested-by:`` will also help the patch to be accepted.
+
+#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
+ it may receive a ``nack``.
+ In this case you will need to make a more convincing technical argument in favor of your patches.
+
+#. In addition a patch will not be accepted if it doesn't address comments from a previous version with fixes or
+ valid arguments.
+
+#. Acked patches will be merged in the current or next merge window.
--
2.5.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2] doc: add contributors guide
2015-12-14 20:32 ` [dpdk-dev] [PATCH v2] " John McNamara
@ 2015-12-14 20:46 ` Mcnamara, John
0 siblings, 0 replies; 14+ messages in thread
From: Mcnamara, John @ 2015-12-14 20:46 UTC (permalink / raw)
To: dev
> -----Original Message-----
> From: Mcnamara, John
> Sent: Monday, December 14, 2015 8:33 PM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; Mcnamara, John
> Subject: [PATCH v2] doc: add contributors guide
>
> Add a document to explain the DPDK patch submission and review process.
>
> Signed-off-by: John McNamara <john.mcnamara@intel.com>
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
Self nack. Wrong version number and annotations.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v5] doc: add contributors guide
2015-12-14 20:45 ` [dpdk-dev] [PATCH v5] " John McNamara
@ 2015-12-14 22:15 ` Thomas Monjalon
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2015-12-14 22:15 UTC (permalink / raw)
To: John McNamara; +Cc: dev
2015-12-14 20:45, John McNamara:
> Add a document to explain the DPDK patch submission and review process.
>
> Signed-off-by: John McNamara <john.mcnamara@intel.com>
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
Applied, thanks
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-12-14 22:16 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-15 16:51 [dpdk-dev] [PATCH] add a dpdk contributors guide John McNamara
2015-10-15 16:51 ` [dpdk-dev] [PATCH] doc: add " John McNamara
2015-10-15 21:36 ` Thomas Monjalon
2015-10-20 10:58 ` Mcnamara, John
2015-10-20 11:03 ` [dpdk-dev] [PATCH v2] " John McNamara
2015-10-23 10:18 ` [dpdk-dev] [PATCH v3] " John McNamara
2015-11-18 13:38 ` Van Haaren, Harry
2015-11-20 12:46 ` [dpdk-dev] [PATCH v4] " John McNamara
2015-12-13 22:35 ` Thomas Monjalon
2015-12-14 9:31 ` Mcnamara, John
2015-12-14 20:32 ` [dpdk-dev] [PATCH v2] " John McNamara
2015-12-14 20:46 ` Mcnamara, John
2015-12-14 20:45 ` [dpdk-dev] [PATCH v5] " John McNamara
2015-12-14 22:15 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).