| Message ID | 20221117172532.538149-7-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | testing and doc updates (pre-PR) | expand |
On 17/11/22 18:25, 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> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Message-Id: <20221111145529.4020801-8-alex.bennee@linaro.org> > --- > docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++----------- > 1 file changed, 49 insertions(+), 26 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Hi Alex, On 17/11/22 18:25, 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> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Message-Id: <20221111145529.4020801-8-alex.bennee@linaro.org> > --- > docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++----------- > 1 file changed, 49 insertions(+), 26 deletions(-) > @@ -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). Revisiting this patch, asking for some real name instead of alias was at least helpful during patch review, we could address the contributor by its name. Addressing an acronym is socially weird (at least in my culture netiquette). > +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.
Ping? On 5/7/23 13:44, Philippe Mathieu-Daudé wrote: > Hi Alex, > > On 17/11/22 18:25, 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> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> Message-Id: <20221111145529.4020801-8-alex.bennee@linaro.org> >> --- >> docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++----------- >> 1 file changed, 49 insertions(+), 26 deletions(-) > > >> @@ -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). > > Revisiting this patch, asking for some real name instead of alias > was at least helpful during patch review, we could address the > contributor by its name. Addressing an acronym is socially weird > (at least in my culture netiquette). > >> +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.
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > Ping? > > On 5/7/23 13:44, Philippe Mathieu-Daudé wrote: >> Hi Alex, >> On 17/11/22 18:25, 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> >>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >>> Message-Id: <20221111145529.4020801-8-alex.bennee@linaro.org> >>> --- >>> docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++----------- >>> 1 file changed, 49 insertions(+), 26 deletions(-) >> >>> @@ -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). >> Revisiting this patch, asking for some real name instead of alias >> was at least helpful during patch review, we could address the >> contributor by its name. Addressing an acronym is socially weird >> (at least in my culture netiquette). Is it that weird? We use nicks all the time on IRC. The only driver for real names for the signoff is its harder to have confidence the contribution is valid because you might not be able to find who is behind an anonymous nick if something comes up later.
On Fri, Sep 01, 2023 at 11:08:15AM +0100, Alex Bennée wrote: > > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > > > Ping? > > > > On 5/7/23 13:44, Philippe Mathieu-Daudé wrote: > >> Hi Alex, > >> On 17/11/22 18:25, 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> > >>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > >>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > >>> Message-Id: <20221111145529.4020801-8-alex.bennee@linaro.org> > >>> --- > >>> docs/devel/submitting-a-patch.rst | 75 ++++++++++++++++++++----------- > >>> 1 file changed, 49 insertions(+), 26 deletions(-) > >> > >>> @@ -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). > >> Revisiting this patch, asking for some real name instead of alias > >> was at least helpful during patch review, we could address the > >> contributor by its name. Addressing an acronym is socially weird > >> (at least in my culture netiquette). > > Is it that weird? We use nicks all the time on IRC. > > The only driver for real names for the signoff is its harder to have > confidence the contribution is valid because you might not be able to > find who is behind an anonymous nick if something comes up later. The Signed-off-by is intended as a legal attestation of permission to contribute. Having the signoff use an obviously anonymous nick could be said to undermine the legal value of the Signed-off-by. That's not to say something that /looks/ like a real name is actually the persons real name - we can't prove that without ID checks which we're not going to do. Something that looks like a real name is at least plausible to accept as a persons' real world identity, where as a nick is clearly just an online persona where there is an explicit intent to hide the real identify. This kind of distinction / intent often matters in the legal arena. With regards, Daniel
diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst index 9c7c4331f3..1f2bde0625 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: Real 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 therefore 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: