The recently introduced Co-developed-by: tag has seen a spate of
improper usage, likely because the tag is less intuitive and slightly
more complex than most other tags. Rework the documentation to be more
explicit in how Co-developed-by: is intended to be used, add examples
in the docs, and add logic to checkpatch to reinforce proper usage.
v1: https://lkml.kernel.org/r/[email protected]
v2: https://lkml.kernel.org/r/[email protected]
Rewrite the blurb to state standard sign-off procedure should be
followed as opposed to dictating the original author's SOB be last.
v3: https://lkml.kernel.org/r/[email protected]
Update a similar blurb in Documentation/process/5.Posting.rst
v4: https://lkml.kernel.org/r/[email protected]
Rework the blurbs to avoid use of the word "original" [Tobin]
v5: Add checkpatch warnings [Joe]
Fix a spelling error [Niklas]
Sean Christopherson (2):
docs: Clarify the usage and sign-off requirements for Co-developed-by
checkpatch: Warn on improper usage of Co-developed-by
Documentation/process/5.Posting.rst | 10 +++--
Documentation/process/submitting-patches.rst | 40 +++++++++++++++++---
scripts/checkpatch.pl | 18 +++++++++
3 files changed, 59 insertions(+), 9 deletions(-)
--
2.21.0
The documentation for Co-developed-by is a bit light on details, e.g. it
doesn't explicitly state that:
- Multiple Co-developed-by tags are perfectly acceptable
- Co-developed-by and Signed-off-by must be paired together
- SOB ordering should still follow standard sign-off procedure
Lack of explicit direction has resulted in developers taking a variety
of approaches, often lacking any intent whatsoever, e.g. scattering SOBs
willy-nilly, collecting them all at the end or the beginning, etc...
Tweak the wording to make it clear that multiple co-authors are allowed,
and document the expectation that standard sign-off procedures are to
be followed.
The use of "original author" has also led to confusion as many patches
don't have just one "original" author, e.g. when multiple developers
are involved from the genesis of the patch. Remove all usage of
"original" and instead call out that Co-developed-by is simply a way to
provide attribution in addition to the From tag, i.e. neither tag is
intended to imply anything with regard to who did what.
Provide examples to (hopefully) eliminate any ambiguity.
Cc: Tobin C. Harding <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Jorge Ramirez-Ortiz <[email protected]>
Cc: Jonathan Cameron <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Niklas Cassel <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
Documentation/process/5.Posting.rst | 10 +++--
Documentation/process/submitting-patches.rst | 40 +++++++++++++++++---
2 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
index 4213e580f273..855a70b80269 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -216,10 +216,12 @@ The tags in common use are:
which can be found in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
Code without a proper signoff cannot be merged into the mainline.
- - Co-developed-by: states that the patch was also created by another developer
- along with the original author. This is useful at times when multiple
- people work on a single patch. Note, this person also needs to have a
- Signed-off-by: line in the patch as well.
+ - Co-developed-by: states that the patch was co-created by several developers;
+ it is a used to give attribution to co-authors (in addition to the author
+ attributed by the From: tag) when multiple people work on a single patch.
+ Every Co-developed-by: must be immediately followed by a Signed-off-by: of
+ the associated co-author. Details and examples can be found in
+ :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`.
- Acked-by: indicates an agreement by another developer (often a
maintainer of the relevant code) that the patch is appropriate for
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index be7d1829c3af..06db26b12495 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -545,10 +545,40 @@ person it names - but it should indicate that this person was copied on the
patch. This tag documents that potentially interested parties
have been included in the discussion.
-A Co-developed-by: states that the patch was also created by another developer
-along with the original author. This is useful at times when multiple people
-work on a single patch. Note, this person also needs to have a Signed-off-by:
-line in the patch as well.
+Co-developed-by: states that the patch was co-created by multiple developers;
+it is a used to give attribution to co-authors (in addition to the author
+attributed by the From: tag) when several people work on a single patch. Since
+Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
+followed by a Signed-off-by: of the associated co-author. Standard sign-off
+procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
+chronological history of the patch insofar as possible, regardless of whether
+the author is attributed via From: or Co-developed-by:. Notably, the last
+Signed-off-by: must always be that of the developer submitting the patch.
+
+Note, the From: tag is optional when the From: author is also the person (and
+email) listed in the From: line of the email header.
+
+Example of a patch submitted by the From: author::
+
+ <changelog>
+
+ Co-developed-by: First Co-Author <[email protected]>
+ Signed-off-by: First Co-Author <[email protected]>
+ Co-developed-by: Second Co-Author <[email protected]>
+ Signed-off-by: Second Co-Author <[email protected]>
+ Signed-off-by: From Author <[email protected]>
+
+Example of a patch submitted by a Co-developed-by: author::
+
+ From: From Author <[email protected]>
+
+ <changelog>
+
+ Co-developed-by: Random Co-Author <[email protected]>
+ Signed-off-by: Random Co-Author <[email protected]>
+ Signed-off-by: From Author <[email protected]>
+ Co-developed-by: Submitting Co-Author <[email protected]>
+ Signed-off-by: Submitting Co-Author <[email protected]>
13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
@@ -696,7 +726,7 @@ A couple of example Subjects::
The ``from`` line must be the very first line in the message body,
and has the form:
- From: Original Author <[email protected]>
+ From: Patch Author <[email protected]>
The ``from`` line specifies who will be credited as the author of the
patch in the permanent changelog. If the ``from`` line is missing,
--
2.21.0
The purpose of Co-developed-by: is to give attribution to authors who
aren't already attributed by the From: tag, i.e. who aren't the nominal
patch author. Because Co-developed-by: is essentially a variation of
From:, it must be accompanied by a Signed-off-by: of the associated
co-author. To ease the burden of determining whether or not co-authors
have signed off, Co-developed-by and Signed-off-by: must be explicitly
paired, i.e. on consecutive lines for a given co-author.
Suggested-by: Joe Perches <[email protected]>
Cc: Tobin C. Harding <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Jorge Ramirez-Ortiz <[email protected]>
Cc: Jonathan Cameron <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Niklas Cassel <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
scripts/checkpatch.pl | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5b756278df13..bbe71f017f6d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2687,6 +2687,24 @@ sub process {
} else {
$signatures{$sig_nospace} = 1;
}
+
+# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email
+ if ($sign_off =~ /^co-developed-by:$/i) {
+ if ($email eq $author) {
+ WARN("BAD_SIGN_OFF",
+ "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . "$here\n" . $rawline);
+ }
+ if (!defined $lines[$linenr]) {
+ WARN("BAD_SIGN_OFF",
+ "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline);
+ } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) {
+ WARN("BAD_SIGN_OFF",
+ "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]);
+ } elsif ($1 ne $email) {
+ WARN("BAD_SIGN_OFF",
+ "Co-developed-by and Signed-off-by: name/email do not match \n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]);
+ }
+ }
}
# Check email subject for common tools that don't need to be mentioned
--
2.21.0
On Fri, Mar 22, 2019 at 02:11:36PM -0700, Sean Christopherson wrote:
> The documentation for Co-developed-by is a bit light on details, e.g. it
> doesn't explicitly state that:
>
> - Multiple Co-developed-by tags are perfectly acceptable
> - Co-developed-by and Signed-off-by must be paired together
> - SOB ordering should still follow standard sign-off procedure
>
> Lack of explicit direction has resulted in developers taking a variety
> of approaches, often lacking any intent whatsoever, e.g. scattering SOBs
> willy-nilly, collecting them all at the end or the beginning, etc...
> Tweak the wording to make it clear that multiple co-authors are allowed,
> and document the expectation that standard sign-off procedures are to
> be followed.
>
> The use of "original author" has also led to confusion as many patches
> don't have just one "original" author, e.g. when multiple developers
> are involved from the genesis of the patch. Remove all usage of
> "original" and instead call out that Co-developed-by is simply a way to
> provide attribution in addition to the From tag, i.e. neither tag is
> intended to imply anything with regard to who did what.
>
> Provide examples to (hopefully) eliminate any ambiguity.
>
> Cc: Tobin C. Harding <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Jorge Ramirez-Ortiz <[email protected]>
> Cc: Jonathan Cameron <[email protected]>
> Cc: Joe Perches <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Niklas Cassel <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> Documentation/process/5.Posting.rst | 10 +++--
> Documentation/process/submitting-patches.rst | 40 +++++++++++++++++---
> 2 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
> index 4213e580f273..855a70b80269 100644
> --- a/Documentation/process/5.Posting.rst
> +++ b/Documentation/process/5.Posting.rst
> @@ -216,10 +216,12 @@ The tags in common use are:
> which can be found in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
> Code without a proper signoff cannot be merged into the mainline.
>
> - - Co-developed-by: states that the patch was also created by another developer
> - along with the original author. This is useful at times when multiple
> - people work on a single patch. Note, this person also needs to have a
> - Signed-off-by: line in the patch as well.
> + - Co-developed-by: states that the patch was co-created by several developers;
> + it is a used to give attribution to co-authors (in addition to the author
> + attributed by the From: tag) when multiple people work on a single patch.
> + Every Co-developed-by: must be immediately followed by a Signed-off-by: of
> + the associated co-author. Details and examples can be found in
> + :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`.
>
> - Acked-by: indicates an agreement by another developer (often a
> maintainer of the relevant code) that the patch is appropriate for
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index be7d1829c3af..06db26b12495 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -545,10 +545,40 @@ person it names - but it should indicate that this person was copied on the
> patch. This tag documents that potentially interested parties
> have been included in the discussion.
>
> -A Co-developed-by: states that the patch was also created by another developer
> -along with the original author. This is useful at times when multiple people
> -work on a single patch. Note, this person also needs to have a Signed-off-by:
> -line in the patch as well.
> +Co-developed-by: states that the patch was co-created by multiple developers;
> +it is a used to give attribution to co-authors (in addition to the author
> +attributed by the From: tag) when several people work on a single patch. Since
> +Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
> +followed by a Signed-off-by: of the associated co-author. Standard sign-off
> +procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
> +chronological history of the patch insofar as possible, regardless of whether
> +the author is attributed via From: or Co-developed-by:. Notably, the last
> +Signed-off-by: must always be that of the developer submitting the patch.
> +
> +Note, the From: tag is optional when the From: author is also the person (and
> +email) listed in the From: line of the email header.
> +
> +Example of a patch submitted by the From: author::
> +
> + <changelog>
> +
> + Co-developed-by: First Co-Author <[email protected]>
> + Signed-off-by: First Co-Author <[email protected]>
> + Co-developed-by: Second Co-Author <[email protected]>
> + Signed-off-by: Second Co-Author <[email protected]>
> + Signed-off-by: From Author <[email protected]>
> +
> +Example of a patch submitted by a Co-developed-by: author::
> +
> + From: From Author <[email protected]>
> +
> + <changelog>
> +
> + Co-developed-by: Random Co-Author <[email protected]>
> + Signed-off-by: Random Co-Author <[email protected]>
> + Signed-off-by: From Author <[email protected]>
> + Co-developed-by: Submitting Co-Author <[email protected]>
> + Signed-off-by: Submitting Co-Author <[email protected]>
>
>
> 13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
> @@ -696,7 +726,7 @@ A couple of example Subjects::
> The ``from`` line must be the very first line in the message body,
> and has the form:
>
> - From: Original Author <[email protected]>
> + From: Patch Author <[email protected]>
>
> The ``from`` line specifies who will be credited as the author of the
> patch in the permanent changelog. If the ``from`` line is missing,
> --
> 2.21.0
>
Crystal clear now, thanks.
Tobin
On Fri, Mar 22, 2019 at 02:11:37PM -0700, Sean Christopherson wrote:
> The purpose of Co-developed-by: is to give attribution to authors who
> aren't already attributed by the From: tag, i.e. who aren't the nominal
> patch author. Because Co-developed-by: is essentially a variation of
> From:, it must be accompanied by a Signed-off-by: of the associated
> co-author. To ease the burden of determining whether or not co-authors
> have signed off, Co-developed-by and Signed-off-by: must be explicitly
> paired, i.e. on consecutive lines for a given co-author.
>
> Suggested-by: Joe Perches <[email protected]>
> Cc: Tobin C. Harding <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Jorge Ramirez-Ortiz <[email protected]>
> Cc: Jonathan Cameron <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Niklas Cassel <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> scripts/checkpatch.pl | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 5b756278df13..bbe71f017f6d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2687,6 +2687,24 @@ sub process {
> } else {
> $signatures{$sig_nospace} = 1;
> }
> +
> +# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email
> + if ($sign_off =~ /^co-developed-by:$/i) {
> + if ($email eq $author) {
> + WARN("BAD_SIGN_OFF",
> + "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . "$here\n" . $rawline);
> + }
> + if (!defined $lines[$linenr]) {
> + WARN("BAD_SIGN_OFF",
> + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline);
> + } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) {
> + WARN("BAD_SIGN_OFF",
> + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]);
> + } elsif ($1 ne $email) {
> + WARN("BAD_SIGN_OFF",
> + "Co-developed-by and Signed-off-by: name/email do not match \n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]);
> + }
> + }
> }
>
> # Check email subject for common tools that don't need to be mentioned
> --
> 2.21.0
>
Works as advertised!
thanks,
Tobin.
On Fri, 22 Mar 2019 14:11:36 -0700
Sean Christopherson <[email protected]> wrote:
> The documentation for Co-developed-by is a bit light on details, e.g. it
> doesn't explicitly state that:
>
> - Multiple Co-developed-by tags are perfectly acceptable
> - Co-developed-by and Signed-off-by must be paired together
> - SOB ordering should still follow standard sign-off procedure
>
> Lack of explicit direction has resulted in developers taking a variety
> of approaches, often lacking any intent whatsoever, e.g. scattering SOBs
> willy-nilly, collecting them all at the end or the beginning, etc...
> Tweak the wording to make it clear that multiple co-authors are allowed,
> and document the expectation that standard sign-off procedures are to
> be followed.
>
> The use of "original author" has also led to confusion as many patches
> don't have just one "original" author, e.g. when multiple developers
> are involved from the genesis of the patch. Remove all usage of
> "original" and instead call out that Co-developed-by is simply a way to
> provide attribution in addition to the From tag, i.e. neither tag is
> intended to imply anything with regard to who did what.
>
> Provide examples to (hopefully) eliminate any ambiguity.
I've applied this. Joe, do you want to handle the checkpatch.pl patch, or
would you prefer I take that one too?
Thanks,
jon
On Mon, 2019-03-25 at 10:31 -0600, Jonathan Corbet wrote:
> On Fri, 22 Mar 2019 14:11:36 -0700 Sean Christopherson <[email protected]> wrote:
> > The documentation for Co-developed-by is a bit light on details,
[]
> I've applied this. Joe, do you want to handle the checkpatch.pl patch, or
> would you prefer I take that one too?
You taking it is fine with me.
On Mon, 25 Mar 2019 09:40:32 -0700
Joe Perches <[email protected]> wrote:
> > I've applied this. Joe, do you want to handle the checkpatch.pl patch, or
> > would you prefer I take that one too?
>
> You taking it is fine with me.
>
OK, I've now done that, thanks.
jon