diff mbox series

[RFC,3/4] docs/devel: simplify the minimal checklist

Message ID 20221012121152.1179051-4-alex.bennee@linaro.org
State Superseded
Headers show
Series docs/devel suggestions for discussion | expand

Commit Message

Alex Bennée Oct. 12, 2022, 12:11 p.m. UTC
The bullet points are quite long and contain process tips. Move those
bits of the bullet to the relevant sections and link to them. Use a
table for nicer formatting of the checklist.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++-----------
 roms/qboot                        |  2 +-
 2 files changed, 50 insertions(+), 27 deletions(-)

Comments

Daniel P. Berrangé Oct. 12, 2022, 12:14 p.m. UTC | #1
On Wed, Oct 12, 2022 at 01:11:51PM +0100, Alex Bennée wrote:
> The bullet points are quite long and contain process tips. Move those
> bits of the bullet to the relevant sections and link to them. Use a
> table for nicer formatting of the checklist.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++-----------
>  roms/qboot                        |  2 +-

git submodule surprise !


With regards,
Daniel
Stefan Hajnoczi Oct. 12, 2022, 3:02 p.m. UTC | #2
On Wed, Oct 12, 2022 at 01:11:51PM +0100, Alex Bennée wrote:
> The bullet points are quite long and contain process tips. Move those
> bits of the bullet to the relevant sections and link to them. Use a
> table for nicer formatting of the checklist.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++-----------
>  roms/qboot                        |  2 +-
>  2 files changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst
> index fb1673e974..41771501bf 100644
> --- a/docs/devel/submitting-a-patch.rst
> +++ b/docs/devel/submitting-a-patch.rst
> @@ -12,25 +12,18 @@ be committed faster.
>  This page seems very long, so if you are only trying to post a quick
>  one-shot fix, the bare minimum we ask is that:
>  
> --  You **must** provide a Signed-off-by: line (this is a hard
> -   requirement because it's how you say "I'm legally okay to contribute
> -   this and happy for it to go into QEMU", modeled after the `Linux kernel
> -   <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
> -   policy.) ``git commit -s`` or ``git format-patch -s`` will add one.
> --  All contributions to QEMU must be **sent as patches** to the
> -   qemu-devel `mailing list <https://wiki.qemu.org/Contribute/MailingLists>`__.
> -   Patch contributions should not be posted on the bug tracker, posted on
> -   forums, or externally hosted and linked to. (We have other mailing lists too,
> -   but all patches must go to qemu-devel, possibly with a Cc: to another
> -   list.) ``git send-email`` (`step-by-step setup
> -   guide <https://git-send-email.io/>`__ and `hints and
> -   tips <https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst>`__)
> -   works best for delivering the patch without mangling it, but
> -   attachments can be used as a last resort on a first-time submission.
> --  You must read replies to your message, and be willing to act on them.
> -   Note, however, that maintainers are often willing to manually fix up
> -   first-time contributions, since there is a learning curve involved in
> -   making an ideal patch submission.
> +.. list-table:: Minimal Checklist for Patches
> +   :widths: 35 65
> +   :header-rows: 1
> +
> +   * - Check
> +     - Reason
> +   * - Patches contain Signed-off-by: Author Name <author@email>
> +     - States you are legally able to contribute the code. See :ref:`patch_emails_must_include_a_signed_off_by_line`
> +   * - Sent as patch emails to ``qemu-devel@nongnu.org``
> +     - The project uses an email list based workflow. See :ref:`submitting_your_patches`
> +   * - Be prepared to respond to review comments
> +     - Code that doesn't pass review will not get merged. See :ref:`participating_in_code_review`
>  
>  You do not have to subscribe to post (list policy is to reply-to-all to
>  preserve CCs and keep non-subscribers in the loop on the threads they
> @@ -229,6 +222,19 @@ bisection doesn't land on a known-broken state.
>  Submitting your Patches
>  -----------------------
>  
> +The QEMU project uses a public email based workflow for reviewing and
> +merging patches. As a result all contributions to QEMU must be **sent
> +as patches** to the qemu-devel `mailing list
> +<https://wiki.qemu.org/Contribute/MailingLists>`__. Patch
> +contributions should not be posted on the bug tracker, posted on
> +forums, or externally hosted and linked to. (We have other mailing
> +lists too, but all patches must go to qemu-devel, possibly with a Cc:
> +to another list.) ``git send-email`` (`step-by-step setup guide
> +<https://git-send-email.io/>`__ and `hints and tips
> +<https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst>`__)
> +works best for delivering the patch without mangling it, but
> +attachments can be used as a last resort on a first-time submission.
> +
>  .. _if_you_cannot_send_patch_emails:
>  
>  If you cannot send patch emails
> @@ -314,10 +320,12 @@ git repository to fetch the original commit.
>  Patch emails must include a ``Signed-off-by:`` line
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> -For more information see `SubmittingPatches 1.12
> -<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
> -This is vital or we will not be able to apply your patch! Please use
> -your real name to sign a patch (not an alias or acronym).
> +Your patches **must** include a Signed-off-by: line. This is a hard
> +requirement because it's how you say "I'm legally okay to contribute
> +this and happy for it to go into QEMU". The process is modelled after
> +the `Linux kernel
> +<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
> +policy.
>  
>  If you wrote the patch, make sure your "From:" and "Signed-off-by:"
>  lines use the same spelling. It's okay if you subscribe or contribute to
> @@ -327,6 +335,11 @@ include a "From:" line in the body of the email (different from your
>  envelope From:) that will give credit to the correct author; but again,
>  that author's Signed-off-by: line is mandatory, with the same spelling.
>  
> +There are various tooling options for automatically adding these tags
> +include using ``git commit -s`` or ``git format-patch -s``. For more
> +information see `SubmittingPatches 1.12
> +<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
> +
>  .. _include_a_meaningful_cover_letter:
>  
>  Include a meaningful cover letter
> @@ -397,9 +410,19 @@ Participating in Code Review
>  ----------------------------
>  
>  All patches submitted to the QEMU project go through a code review
> -process before they are accepted. Some areas of code that are well
> -maintained may review patches quickly, lesser-loved areas of code may
> -have a longer delay.
> +process before they are accepted. This will often mean a series will
> +go through a number of iterations before being picked up by
> +:ref:`maintainers<maintainers>`. You therefor should be prepared to

therefore

> +read replies to your messages and be willing to act on them.
> +
> +Maintainers are often willing to manually fix up first-time
> +contributions, since there is a learning curve involved in making an
> +ideal patch submission. However for the best results you should
> +proactively respond to suggestions with changes or justifications for
> +your current approach.
> +
> +Some areas of code that are well maintained may review patches
> +quickly, lesser-loved areas of code may have a longer delay.
>  
>  .. _stay_around_to_fix_problems_raised_in_code_review:
Mark Cave-Ayland Oct. 14, 2022, 9:31 a.m. UTC | #3
On 12/10/2022 13:11, Alex Bennée wrote:

> The bullet points are quite long and contain process tips. Move those
> bits of the bullet to the relevant sections and link to them. Use a
> table for nicer formatting of the checklist.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++-----------
>   roms/qboot                        |  2 +-
>   2 files changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst
> index fb1673e974..41771501bf 100644
> --- a/docs/devel/submitting-a-patch.rst
> +++ b/docs/devel/submitting-a-patch.rst
> @@ -12,25 +12,18 @@ be committed faster.
>   This page seems very long, so if you are only trying to post a quick
>   one-shot fix, the bare minimum we ask is that:
>   
> --  You **must** provide a Signed-off-by: line (this is a hard
> -   requirement because it's how you say "I'm legally okay to contribute
> -   this and happy for it to go into QEMU", modeled after the `Linux kernel
> -   <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
> -   policy.) ``git commit -s`` or ``git format-patch -s`` will add one.
> --  All contributions to QEMU must be **sent as patches** to the
> -   qemu-devel `mailing list <https://wiki.qemu.org/Contribute/MailingLists>`__.
> -   Patch contributions should not be posted on the bug tracker, posted on
> -   forums, or externally hosted and linked to. (We have other mailing lists too,
> -   but all patches must go to qemu-devel, possibly with a Cc: to another
> -   list.) ``git send-email`` (`step-by-step setup
> -   guide <https://git-send-email.io/>`__ and `hints and
> -   tips <https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst>`__)
> -   works best for delivering the patch without mangling it, but
> -   attachments can be used as a last resort on a first-time submission.
> --  You must read replies to your message, and be willing to act on them.
> -   Note, however, that maintainers are often willing to manually fix up
> -   first-time contributions, since there is a learning curve involved in
> -   making an ideal patch submission.
> +.. list-table:: Minimal Checklist for Patches
> +   :widths: 35 65
> +   :header-rows: 1
> +
> +   * - Check
> +     - Reason
> +   * - Patches contain Signed-off-by: Author Name <author@email>
> +     - States you are legally able to contribute the code. See :ref:`patch_emails_must_include_a_signed_off_by_line`
> +   * - Sent as patch emails to ``qemu-devel@nongnu.org``
> +     - The project uses an email list based workflow. See :ref:`submitting_your_patches`
> +   * - Be prepared to respond to review comments
> +     - Code that doesn't pass review will not get merged. See :ref:`participating_in_code_review`
>   
>   You do not have to subscribe to post (list policy is to reply-to-all to
>   preserve CCs and keep non-subscribers in the loop on the threads they
> @@ -229,6 +222,19 @@ bisection doesn't land on a known-broken state.
>   Submitting your Patches
>   -----------------------
>   
> +The QEMU project uses a public email based workflow for reviewing and
> +merging patches. As a result all contributions to QEMU must be **sent
> +as patches** to the qemu-devel `mailing list
> +<https://wiki.qemu.org/Contribute/MailingLists>`__. Patch
> +contributions should not be posted on the bug tracker, posted on
> +forums, or externally hosted and linked to. (We have other mailing
> +lists too, but all patches must go to qemu-devel, possibly with a Cc:
> +to another list.) ``git send-email`` (`step-by-step setup guide
> +<https://git-send-email.io/>`__ and `hints and tips
> +<https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst>`__)
> +works best for delivering the patch without mangling it, but
> +attachments can be used as a last resort on a first-time submission.
> +
>   .. _if_you_cannot_send_patch_emails:
>   
>   If you cannot send patch emails
> @@ -314,10 +320,12 @@ git repository to fetch the original commit.
>   Patch emails must include a ``Signed-off-by:`` line
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   
> -For more information see `SubmittingPatches 1.12
> -<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
> -This is vital or we will not be able to apply your patch! Please use
> -your real name to sign a patch (not an alias or acronym).

I think it is worth keeping that part about using a real name rather than an 
alias/acronym - perhaps that could be included in the Minimal Checklist table above?

> +Your patches **must** include a Signed-off-by: line. This is a hard
> +requirement because it's how you say "I'm legally okay to contribute
> +this and happy for it to go into QEMU". The process is modelled after
> +the `Linux kernel
> +<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
> +policy.
>   
>   If you wrote the patch, make sure your "From:" and "Signed-off-by:"
>   lines use the same spelling. It's okay if you subscribe or contribute to
> @@ -327,6 +335,11 @@ include a "From:" line in the body of the email (different from your
>   envelope From:) that will give credit to the correct author; but again,
>   that author's Signed-off-by: line is mandatory, with the same spelling.
>   
> +There are various tooling options for automatically adding these tags
> +include using ``git commit -s`` or ``git format-patch -s``. For more
> +information see `SubmittingPatches 1.12
> +<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
> +
>   .. _include_a_meaningful_cover_letter:
>   
>   Include a meaningful cover letter
> @@ -397,9 +410,19 @@ Participating in Code Review
>   ----------------------------
>   
>   All patches submitted to the QEMU project go through a code review
> -process before they are accepted. Some areas of code that are well
> -maintained may review patches quickly, lesser-loved areas of code may
> -have a longer delay.
> +process before they are accepted. This will often mean a series will
> +go through a number of iterations before being picked up by
> +:ref:`maintainers<maintainers>`. You therefor should be prepared to
> +read replies to your messages and be willing to act on them.
> +
> +Maintainers are often willing to manually fix up first-time
> +contributions, since there is a learning curve involved in making an
> +ideal patch submission. However for the best results you should
> +proactively respond to suggestions with changes or justifications for
> +your current approach.
> +
> +Some areas of code that are well maintained may review patches
> +quickly, lesser-loved areas of code may have a longer delay.
>   
>   .. _stay_around_to_fix_problems_raised_in_code_review:
>   
> diff --git a/roms/qboot b/roms/qboot
> index 8ca302e86d..a5300c4949 160000
> --- a/roms/qboot
> +++ b/roms/qboot
> @@ -1 +1 @@
> -Subproject commit 8ca302e86d685fa05b16e2b208888243da319941
> +Subproject commit a5300c4949b8d4de2d34bedfaed66793f48ec948


ATB,

Mark.
diff mbox series

Patch

diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst
index fb1673e974..41771501bf 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -12,25 +12,18 @@  be committed faster.
 This page seems very long, so if you are only trying to post a quick
 one-shot fix, the bare minimum we ask is that:
 
--  You **must** provide a Signed-off-by: line (this is a hard
-   requirement because it's how you say "I'm legally okay to contribute
-   this and happy for it to go into QEMU", modeled after the `Linux kernel
-   <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
-   policy.) ``git commit -s`` or ``git format-patch -s`` will add one.
--  All contributions to QEMU must be **sent as patches** to the
-   qemu-devel `mailing list <https://wiki.qemu.org/Contribute/MailingLists>`__.
-   Patch contributions should not be posted on the bug tracker, posted on
-   forums, or externally hosted and linked to. (We have other mailing lists too,
-   but all patches must go to qemu-devel, possibly with a Cc: to another
-   list.) ``git send-email`` (`step-by-step setup
-   guide <https://git-send-email.io/>`__ and `hints and
-   tips <https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst>`__)
-   works best for delivering the patch without mangling it, but
-   attachments can be used as a last resort on a first-time submission.
--  You must read replies to your message, and be willing to act on them.
-   Note, however, that maintainers are often willing to manually fix up
-   first-time contributions, since there is a learning curve involved in
-   making an ideal patch submission.
+.. list-table:: Minimal Checklist for Patches
+   :widths: 35 65
+   :header-rows: 1
+
+   * - Check
+     - Reason
+   * - Patches contain Signed-off-by: Author Name <author@email>
+     - States you are legally able to contribute the code. See :ref:`patch_emails_must_include_a_signed_off_by_line`
+   * - Sent as patch emails to ``qemu-devel@nongnu.org``
+     - The project uses an email list based workflow. See :ref:`submitting_your_patches`
+   * - Be prepared to respond to review comments
+     - Code that doesn't pass review will not get merged. See :ref:`participating_in_code_review`
 
 You do not have to subscribe to post (list policy is to reply-to-all to
 preserve CCs and keep non-subscribers in the loop on the threads they
@@ -229,6 +222,19 @@  bisection doesn't land on a known-broken state.
 Submitting your Patches
 -----------------------
 
+The QEMU project uses a public email based workflow for reviewing and
+merging patches. As a result all contributions to QEMU must be **sent
+as patches** to the qemu-devel `mailing list
+<https://wiki.qemu.org/Contribute/MailingLists>`__. Patch
+contributions should not be posted on the bug tracker, posted on
+forums, or externally hosted and linked to. (We have other mailing
+lists too, but all patches must go to qemu-devel, possibly with a Cc:
+to another list.) ``git send-email`` (`step-by-step setup guide
+<https://git-send-email.io/>`__ and `hints and tips
+<https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst>`__)
+works best for delivering the patch without mangling it, but
+attachments can be used as a last resort on a first-time submission.
+
 .. _if_you_cannot_send_patch_emails:
 
 If you cannot send patch emails
@@ -314,10 +320,12 @@  git repository to fetch the original commit.
 Patch emails must include a ``Signed-off-by:`` line
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-For more information see `SubmittingPatches 1.12
-<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
-This is vital or we will not be able to apply your patch! Please use
-your real name to sign a patch (not an alias or acronym).
+Your patches **must** include a Signed-off-by: line. This is a hard
+requirement because it's how you say "I'm legally okay to contribute
+this and happy for it to go into QEMU". The process is modelled after
+the `Linux kernel
+<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
+policy.
 
 If you wrote the patch, make sure your "From:" and "Signed-off-by:"
 lines use the same spelling. It's okay if you subscribe or contribute to
@@ -327,6 +335,11 @@  include a "From:" line in the body of the email (different from your
 envelope From:) that will give credit to the correct author; but again,
 that author's Signed-off-by: line is mandatory, with the same spelling.
 
+There are various tooling options for automatically adding these tags
+include using ``git commit -s`` or ``git format-patch -s``. For more
+information see `SubmittingPatches 1.12
+<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
+
 .. _include_a_meaningful_cover_letter:
 
 Include a meaningful cover letter
@@ -397,9 +410,19 @@  Participating in Code Review
 ----------------------------
 
 All patches submitted to the QEMU project go through a code review
-process before they are accepted. Some areas of code that are well
-maintained may review patches quickly, lesser-loved areas of code may
-have a longer delay.
+process before they are accepted. This will often mean a series will
+go through a number of iterations before being picked up by
+:ref:`maintainers<maintainers>`. You therefor should be prepared to
+read replies to your messages and be willing to act on them.
+
+Maintainers are often willing to manually fix up first-time
+contributions, since there is a learning curve involved in making an
+ideal patch submission. However for the best results you should
+proactively respond to suggestions with changes or justifications for
+your current approach.
+
+Some areas of code that are well maintained may review patches
+quickly, lesser-loved areas of code may have a longer delay.
 
 .. _stay_around_to_fix_problems_raised_in_code_review:
 
diff --git a/roms/qboot b/roms/qboot
index 8ca302e86d..a5300c4949 160000
--- a/roms/qboot
+++ b/roms/qboot
@@ -1 +1 @@ 
-Subproject commit 8ca302e86d685fa05b16e2b208888243da319941
+Subproject commit a5300c4949b8d4de2d34bedfaed66793f48ec948