Message-ID: <20190321234125.GB17705@eros.localdomain>
Date:   Fri, 22 Mar 2019 10:41:25 +1100
From:   "Tobin C. Harding" <me@...in.cc>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@...aro.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Joe Perches <joe@...ches.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Niklas Cassel <niklas.cassel@...aro.org>
Subject: Re: [PATCH v3] docs: Clarify the usage and sign-off requirements for
 Co-developed-by

Hi Sean,

Thanks for working on this, I have been struggling to correctly use (and
get the co-author to SOB) the Co-developed-by tag :)

On Thu, Mar 21, 2019 at 01:01:03PM -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 direciton 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.  Provide examples to (hopefully) eliminate any ambiguity.
> 
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@...aro.org>
> Cc: Jonathan Cameron <jic23@...nel.org>
> Cc: Joe Perches <joe@...ches.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Niklas Cassel <niklas.cassel@...aro.org>
> Cc: Jonathan Corbet <corbet@....net>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@...el.com>
> ---
> 
> v1: https://lkml.kernel.org/r/20190320151140.32432-1-sean.j.christopherson@intel.com
> v2: https://lkml.kernel.org/r/20190321184316.8525-1-sean.j.christopherson@intel.com
>     Rewrite the blurb to state standard sign-off procedure should be
>     followed as opposed to dictating the original author's SOB be last.
> v3: Update a similar blurb in Documentation/process/5.Posting.rst
> 
>  Documentation/process/5.Posting.rst          |  8 +++----
>  Documentation/process/submitting-patches.rst | 24 +++++++++++++++++---
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
> index 4213e580f273..c8cac58f0f93 100644
> --- a/Documentation/process/5.Posting.rst
> +++ b/Documentation/process/5.Posting.rst
> @@ -216,10 +216,10 @@ 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 other developer(s)
> +   along with the original author.  Every Co-developed-by: must be accompanied
> +   by a Signed-off-by: of the 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..a7a9da68a384 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -545,10 +545,28 @@ 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
> +A Co-developed-by: states that the patch was co-created by other developer(s)
>  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.
> +work on a single patch.  Every Co-developed-by: must be immediately followed by
> +a Signed-off-by: of the co-author.  Standard sign-off procedure applies, i.e.
> +the ordering of Co-developed-by:/Signed-off-by: pairs should reflect the
> +chronological history of the patch insofar as possible.  Notably, the last
> +Signed-off-by: must always be that of the developer submitting the patch,
> +regardless of whether they are the original author or a co-author.
> +
> +Example of a patch with multiple co-authors, submitted by the original author::
> +
> +	Co-developed-by: First Co-Author <first@...uthor.example.org>
> +	Signed-off-by: First Co-Author <first@...uthor.example.org>
> +	Co-developed-by: Second Co-Author <second@...uthor.example.org>
> +	Signed-off-by: Second Co-Author <second@...uthor.example.org>
> +	Signed-off-by: Original Author <original@...hor.example.org>
> +
> +Example of a patch submitted by a co-author::
> +
> +	Signed-off-by: Original Author <original@...hor.example.org>
> +	Co-developed-by: Submitting Co-Author <sub@...uthor.example.org>
> +	Signed-off-by: Submitting Co-Author <sub@...uthor.example.org>

As someone who has read the docs then tried with difficulty to use this
tag I think we should drop the term 'original' from describing any author.

The reason is that the two versions above do not explicitly show who is
the 'original' author.  This means the docs contain noise that makes the
tag harder to use.  All the tag shows is that one person is submitting a
patch (the last SOB and sender of the patch) AND someone else worked on
it (and agrees that they did).

FWIW we are trying to give attribution, right?  We don't need to
explicitly state who wrote what so I do not believe that 'original' is a
useful thing to be.  Linus 'originally' wrote Linux but that does not
say squat about the state of the code right now :)

thanks,
Tobin.

Powered by blists - more mailing lists