[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen master] MAINTAINERS: Clarify check-in requirements for mixed-author patches



commit c9e4365d34b8aaaa84ef29142e1876338ce4f97a
Author:     George Dunlap <george.dunlap@xxxxxxxxx>
AuthorDate: Mon Dec 5 16:41:39 2022 +0000
Commit:     George Dunlap <george.dunlap@xxxxxxxxx>
CommitDate: Mon Jan 9 16:32:26 2023 +0000

    MAINTAINERS: Clarify check-in requirements for mixed-author patches
    
    There was a question raised recently about the requirements for
    checking in a patch which was originally written by one maintainer,
    then picked up and modified by a second maintainer, and which they now both
    agree should be checked in.
    
    It was proposed that in that case, the following set of tags would suffice:
    
       Signed-off-by: First Author <...>
       Signed-off-by: Second Author <...>
       Reviewed-by: First Author <...>
    
    The rationale was as follows:
    
    1. The patch will be a mix of code, whose copyright is owned by the
    various authors (or the companies they work for).  It's important to
    keep this information around in the event, for instance, of a license
    change or something else requiring knowledge of the copyright owner.
    
    2. The Signed-off-by of the Second Author approves not only their own
    code, but First Author's code; the Reviewed-by of the First Author
    approves not only their own code, but the Second Author's code.  Thus
    all the code has been approved by a maintainer, as well as someone who
    was not the author.
    
    In support of this, several arguments were put forward:
    
    * We shouldn't make it harder for maintainers to get their code in
      than for non-maintainers
    
    * The system we set up should not add pointless bureaucracy; nor
      discourage collaboration; nor encourage contributors to get around
      the rules by dropping important information.  (For instance, by
      removing the first SoB, so that the patch appears to have been
      written entirely by Second Author.)
    
    Concerns were raised about two maintainers from the same company
    colluding to get a patch in from their company; but such maintainers
    could already collude, by working on the patch in secret, and posting
    it publicly with only a single author's SoB, and having the other
    person review it.
    
    There's also something slightly strange about adding "Reviewed-by" to
    code that you've written; but in the end you're reviewing not only the
    code itself, but the final arrangement of it.  There's no need to
    overcomplicate things.
    
    Encode this in MAINTAINERS as follows:
    
    * Refine the wording of requirement #2 in the check-in policy; such
    that *each change* must have approval from someone other than *the
    person who wrote it*.
    
    * Add a paragraph explicitly stating that the multiple-SoB-approval
      system satisfies the requirements, and why.
    
    Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 MAINTAINERS | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 175f10f33f..0e5eba2312 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -120,8 +120,8 @@ must be met:
 
    See below for rules on nested maintainership.
 
-2. It must have appropriate approval from someone other than the
-   submitter.  This can be either:
+2. Each change must have appropriate approval from someone other than
+   the person who wrote it.  This can be either:
 
   a. An Acked-by from a maintainer of the code being touched (a
      co-maintainer if available, or a more general level maintainer if
@@ -150,6 +150,20 @@ give their co-maintainer opportunity to give feedback, 
perhaps
 declaring their intention to check it in without their co-maintainers
 ack a day before doing so.
 
+In the case where two people collaborate on a patch, at least one of
+whom is a maintainer -- typically where one maintainer will do an
+early version of the patch, and another maintainer will pick it up and
+revise it -- there should be two Signed-off-by's and one Acked-by or
+Reviewed-by; with the maintainer who did the most recent change
+sending the patch, and an Acked-by or Reviewed-by coming from the
+maintainer who did not most recently edit the patch.  This satisfies
+the requirement #2 because a) the Signed-off-by of the sender approves
+the final version of the patch; including all parts of the patch that
+the sender did not write b) the Reviewed-by approves the final version
+of the patch, including all patches that the reviewer did not write.
+Thus all code in the patch has been approved by someone who did not
+write it.
+
 Maintainers may choose to override non-maintainer objections in the
 case that consensus can't be reached.
 
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.