[PATCH] Documentation: SubmittingPatches: Add note about Reviewed-by tags

[PATCH] Documentation: SubmittingPatches: Add note about Reviewed-by tags

[Florian Fainelli]

As is now common in a lot of organization having an internal code review
process (be it through Gerritt or other tools), patches extracted from
this review process and submitted to public mailing-lists will have
pre-existing Reviewed-by tags. Add a note about why these tags exists,
and what a maintainer could be doing with those. Some maintainers did
complain before that these tags had to be added when the patches get
submitted to the public, while some just ignored and took the patches
as-is.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/SubmittingPatches | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index d603fa078235..b1b8e39bc5ee 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -582,6 +582,14 @@ reviewers and to inform maintainers of the degree of review which has been
 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.
+The presence of Reviewed-by tags for an initial patch submission are indicative
+of an existing internal review process that may occur at various organizations,
+prior to a mainline kernel submission.  The presence of these tags can give the
+maintainer a good appreciation that somebody has done an internal review
+following the same guidelines as those done on a public mailing-list.
+Maintainers are encouraged to maintain these tags while accepting and merging
+patches, appreciation of these pre-existing Revivewed-by tags is left are their
+own discretion.
 
 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
-- 
2.1.0

Re: [PATCH] Documentation: SubmittingPatches: Add note about Reviewed-by tags

[Anup Patel]

On Fri, Feb 12, 2016 at 7:42 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> As is now common in a lot of organization having an internal code review
> process (be it through Gerritt or other tools), patches extracted from
> this review process and submitted to public mailing-lists will have
> pre-existing Reviewed-by tags. Add a note about why these tags exists,
> and what a maintainer could be doing with those. Some maintainers did
> complain before that these tags had to be added when the patches get
> submitted to the public, while some just ignored and took the patches
> as-is.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  Documentation/SubmittingPatches | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index d603fa078235..b1b8e39bc5ee 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -582,6 +582,14 @@ reviewers and to inform maintainers of the degree of review which has been
>  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.
> +The presence of Reviewed-by tags for an initial patch submission are indicative
> +of an existing internal review process that may occur at various organizations,
> +prior to a mainline kernel submission.  The presence of these tags can give the
> +maintainer a good appreciation that somebody has done an internal review
> +following the same guidelines as those done on a public mailing-list.
> +Maintainers are encouraged to maintain these tags while accepting and merging
> +patches, appreciation of these pre-existing Revivewed-by tags is left are their
> +own discretion.

Typo fix "left are their own discretion" -> "left at their own discretion".

>
>  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
> --
> 2.1.0
>

Regards,
Anup

Re: [PATCH] Documentation: SubmittingPatches: Add note about Reviewed-by tags

[Markus Mayer]

For some odd reason, the first version of this e-mail had an HTML part
and got rejected from the lists. My apologies for the extra e-mail.

On 11 February 2016 at 18:12, Florian Fainelli <f.fainelli@gmail.com> wrote:
> As is now common in a lot of organization having an internal code review
> process (be it through Gerritt or other tools), patches extracted from
> this review process and submitted to public mailing-lists will have
> pre-existing Reviewed-by tags. Add a note about why these tags exists,
> and what a maintainer could be doing with those. Some maintainers did
> complain before that these tags had to be added when the patches get
> submitted to the public, while some just ignored and took the patches
> as-is.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  Documentation/SubmittingPatches | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index d603fa078235..b1b8e39bc5ee 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -582,6 +582,14 @@ reviewers and to inform maintainers of the degree of review which has been
>  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.
> +The presence of Reviewed-by tags for an initial patch submission are indicative
> +of an existing internal review process that may occur at various organizations,
> +prior to a mainline kernel submission.  The presence of these tags can give the
> +maintainer a good appreciation that somebody has done an internal review
> +following the same guidelines as those done on a public mailing-list.
> +Maintainers are encouraged to maintain these tags while accepting and merging
> +patches, appreciation of these pre-existing Revivewed-by tags is left are their
> +own discretion.

Some nit-picking on the last sentence. How about:

While maintainers are encouraged to maintain these tags when accepting
and merging patches, appreciation of these pre-existing Reviewed-by
tags is left at their own discretion.

There's also a typo in "Reviewed-by" in the original sentence.

>  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
> --
> 2.1.0
>

Re: [PATCH] Documentation: SubmittingPatches: Add note about Reviewed-by tags

[Jonathan Corbet]

On Thu, 11 Feb 2016 18:12:58 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> As is now common in a lot of organization having an internal code review
> process (be it through Gerritt or other tools), patches extracted from
> this review process and submitted to public mailing-lists will have
> pre-existing Reviewed-by tags. Add a note about why these tags exists,
> and what a maintainer could be doing with those. Some maintainers did
> complain before that these tags had to be added when the patches get
> submitted to the public, while some just ignored and took the patches
> as-is.

So I'll confess, I'm not quite sold on this one.  This is a document for
people looking to learn about how to submit patches; it is already far
too long, complex, and bureaucratic.  I'm not at all convinced that
adding suggestions for maintainers is appropriate here.

Is there a real problem that this patch is trying to solve?

Thanks,

jon

Re: [PATCH] Documentation: SubmittingPatches: Add note about Reviewed-by tags

[Scott Branden]

Hi Jon,

Comments below

On 16-02-15 09:43 AM, Jonathan Corbet wrote:
> On Thu, 11 Feb 2016 18:12:58 -0800
> Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>> As is now common in a lot of organization having an internal code review
>> process (be it through Gerritt or other tools), patches extracted from
>> this review process and submitted to public mailing-lists will have
>> pre-existing Reviewed-by tags. Add a note about why these tags exists,
>> and what a maintainer could be doing with those. Some maintainers did
>> complain before that these tags had to be added when the patches get
>> submitted to the public, while some just ignored and took the patches
>> as-is.
>
> So I'll confess, I'm not quite sold on this one.  This is a document for
> people looking to learn about how to submit patches; it is already far
> too long, complex, and bureaucratic.

I agree - the patch submission process is too complicated with the email 
review and approval process in place.  If it could be streamlined in any 
way I'm all for it.  But the documentation associated with this process 
is not detailed enough and needs updating whenever possible.  It is too 
vague and leaves things open for interpretation.  This causes different 
Maintainers to have different opinions on the patch submission format 
right now.  When clarification is needed it should be added in the 
documentation.

> I'm not at all convinced that
> adding suggestions for maintainers is appropriate here.
>
> Is there a real problem that this patch is trying to solve?
Yes - the patch submission documentation does not detail what to do with 
<Reviewed-by> fields from internal code reviews of patches.
>
> Thanks,
>
> jon
>
Regards,
  Scott