docs: submitting-patches: describe preserving review/test tags
diff mbox series

Message ID 20201007084306.12591-1-krzk@kernel.org
State New, archived
Headers show
Series
  • docs: submitting-patches: describe preserving review/test tags
Related show

Commit Message

Krzysztof Kozlowski Oct. 7, 2020, 8:43 a.m. UTC
From time to time, the novice kernel contributors do not add Reviewed-by
or Tested-by tags to the next versions of the patches.  Mostly because
they are unaware that responsibility of adding these tags in next
version is on submitter, not maintainer.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 Documentation/process/submitting-patches.rst | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Jonathan Corbet Oct. 9, 2020, 3:38 p.m. UTC | #1
On Wed,  7 Oct 2020 10:43:06 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> From time to time, the novice kernel contributors do not add Reviewed-by
> or Tested-by tags to the next versions of the patches.  Mostly because
> they are unaware that responsibility of adding these tags in next
> version is on submitter, not maintainer.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 58586ffe2808..9752b6311674 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -527,6 +527,13 @@ done on the patch.  Reviewed-by: tags, when supplied by reviewers known to
>  understand the subject area and to perform thorough reviews, will normally
>  increase the likelihood of your patch getting into the kernel.
>  
> +Both Tested-by and Reviewed-by tags, once received on mailing list from tester
> +or reviewer, should be added by author to the applicable patches when sending
> +next versions.  However if the patch is changed in following version, these
> +tags might not be applicable anymore and thus should be removed.  Usually
> +removal of someone's Tested-by or Reviewed-by tags should be mentioned
> +in the patch changelog (after '---' separator).

after *the* "---" separator

This is a bit ambiguous, though, since the point of sending a new version
of a patch is usually that it has changed.  I'm not quite sure how to best
articulate when a patch has changed enough that reviews and such are no
longer applicable...  If nothing else, "if the patch *has changed
substantially*" or something like that?

Thanks,

jon
Krzysztof Kozlowski Oct. 12, 2020, 5:18 p.m. UTC | #2
On Fri, 9 Oct 2020 at 17:38, Jonathan Corbet <corbet@lwn.net> wrote:
>
> On Wed,  7 Oct 2020 10:43:06 +0200
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> > From time to time, the novice kernel contributors do not add Reviewed-by
> > or Tested-by tags to the next versions of the patches.  Mostly because
> > they are unaware that responsibility of adding these tags in next
> > version is on submitter, not maintainer.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> >  Documentation/process/submitting-patches.rst | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> > index 58586ffe2808..9752b6311674 100644
> > --- a/Documentation/process/submitting-patches.rst
> > +++ b/Documentation/process/submitting-patches.rst
> > @@ -527,6 +527,13 @@ done on the patch.  Reviewed-by: tags, when supplied by reviewers known to
> >  understand the subject area and to perform thorough reviews, will normally
> >  increase the likelihood of your patch getting into the kernel.
> >
> > +Both Tested-by and Reviewed-by tags, once received on mailing list from tester
> > +or reviewer, should be added by author to the applicable patches when sending
> > +next versions.  However if the patch is changed in following version, these
> > +tags might not be applicable anymore and thus should be removed.  Usually
> > +removal of someone's Tested-by or Reviewed-by tags should be mentioned
> > +in the patch changelog (after '---' separator).
>
> after *the* "---" separator
>
> This is a bit ambiguous, though, since the point of sending a new version
> of a patch is usually that it has changed.  I'm not quite sure how to best
> articulate when a patch has changed enough that reviews and such are no
> longer applicable...  If nothing else, "if the patch *has changed
> substantially*" or something like that?

Yes, it is ambiguous because different people see the amount of
changes invalidating tags differently. I do not think we could make
the paragraph above stricter and formalize such rule. I'll rework it
to match your choice of wording.

Best regards,
Krzysztof

Patch
diff mbox series

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 58586ffe2808..9752b6311674 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -527,6 +527,13 @@  done on the patch.  Reviewed-by: tags, when supplied by reviewers known to
 understand the subject area and to perform thorough reviews, will normally
 increase the likelihood of your patch getting into the kernel.
 
+Both Tested-by and Reviewed-by tags, once received on mailing list from tester
+or reviewer, should be added by author to the applicable patches when sending
+next versions.  However if the patch is changed in following version, these
+tags might not be applicable anymore and thus should be removed.  Usually
+removal of someone's Tested-by or Reviewed-by tags should be mentioned
+in the patch changelog (after '---' separator).
+
 A Suggested-by: tag indicates that the patch idea is suggested by the person
 named and ensures credit to the person for the idea. Please note that this
 tag should not be added without the reporter's permission, especially if the