[PATCH] docs: patches: Add a note about reviews and contacting developers

Peter Krempa posted 1 patch 1 year, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/e26c31fa18672aa13d8aacb3a6f519f760e63c04.1653658572.git.pkrempa@redhat.com
docs/submitting-patches.rst | 15 +++++++++++++++
1 file changed, 15 insertions(+)
[PATCH] docs: patches: Add a note about reviews and contacting developers
Posted by Peter Krempa 1 year, 11 months ago
Add a note outling best practices around review and responding to it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 docs/submitting-patches.rst | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/docs/submitting-patches.rst b/docs/submitting-patches.rst
index 7cb5c2e172..d6bc9b623c 100644
--- a/docs/submitting-patches.rst
+++ b/docs/submitting-patches.rst
@@ -89,3 +89,18 @@ Moreover, such patch needs to be prefixed correctly with
 ``--subject-prefix=PATCHv2`` appended to
 ``git send-email`` (substitute ``v2`` with the
 correct version if needed though).
+
+Reviews
+-------
+
+Please note that reviewing may take a lot of effort so be patient. Try to react
+to questions from reviewers. There's no need to acknowledge individual points
+before addressing them in next version. Make sure to address all of reviewers
+requests, or start a discussion if you don't agree with reviewers points.
+
+Feel free to respond to any flaw in any patch on the list, do not feel obliged
+to review the rest of the series in such case.
+
+If your patches don't get attention after some time (~week) feel free to
+remind us on list that your patches still need review. **Do not** contact
+individual maintainers directly to review your patches.
-- 
2.35.3
Re: [PATCH] docs: patches: Add a note about reviews and contacting developers
Posted by Erik Skultety 1 year, 11 months ago
On Fri, May 27, 2022 at 03:36:12PM +0200, Peter Krempa wrote:
> Add a note outling best practices around review and responding to it.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  docs/submitting-patches.rst | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/docs/submitting-patches.rst b/docs/submitting-patches.rst
> index 7cb5c2e172..d6bc9b623c 100644
> --- a/docs/submitting-patches.rst
> +++ b/docs/submitting-patches.rst
> @@ -89,3 +89,18 @@ Moreover, such patch needs to be prefixed correctly with
>  ``--subject-prefix=PATCHv2`` appended to
>  ``git send-email`` (substitute ``v2`` with the
>  correct version if needed though).
> +
> +Reviews
> +-------
> +
> +Please note that reviewing may take a lot of effort so be patient. Try to react

Nit: I'd start the sentences with "Note" and end it with "so please be
patient"

"Trying" to respond to questions may not result in the desired outcome, so I'd
make it more explicit - "Respond to reviewer's questions"

> +to questions from reviewers. There's no need to acknowledge individual points
> +before addressing them in next version. Make sure to address all of reviewers
> +requests, or start a discussion if you don't agree with reviewers points.
> +
> +Feel free to respond to any flaw in any patch on the list, do not feel obliged
> +to review the rest of the series in such case.

^This paragraph is a note for the reviewers...

> +
> +If your patches don't get attention after some time (~week) feel free to
> +remind us on list that your patches still need review. **Do not** contact
> +individual maintainers directly to review your patches.

...while this one is again a note for contributors, IOW they should not be
split.

Overall I like the idea, but I think it would be better if we formatted this as
a list of "Dos and Don'ts" (In fact we can even name the section this way).
You'd end up with something like:

- **do** be patient with reviewers, reviews take a lot of effort and hence may
  take some time
- **do** respond to reviewer's questions
- **don't** contact individual maintainers/developers directly with your
  patches
- **do** remind us of your patches on the list if they haven't gotten any
  attention for a prolonged period (> week) by replying to your patches with a
  "ping"

Notes for reviewers:
- **don't** feel obliged to do a full patch review if you spot a serious flaw
  anywhere in the series


Take ^^^this just as an alternative idea of the same concept.

Erik