[Xen-changelog] [xen master] MAINTAINERS: Add explicit check-in policy section

[patchbot@xxxxxxx]

commit 6c47c37b9b40d6fe40bce8c8fd39135f6d549c8c
Author:     George Dunlap <george.dunlap@xxxxxxxxxx>
AuthorDate: Thu Dec 5 17:19:47 2019 +0000
Commit:     George Dunlap <george.dunlap@xxxxxxxxxx>
CommitDate: Tue Feb 11 15:21:19 2020 +0000

    MAINTAINERS: Add explicit check-in policy section
    
    The "nesting" section in the MAINTAINERS file was not initially
    intended to describe the check-in policy for patches, but only how
    nesting worked; but since there was no check-in policy, it has been
    acting as a de-facto policy.
    
    One problem with this is that the policy is not complete: It doesn't
    cover open objections, time to check-in, or so on.  The other problem
    with the policy is that, as written, it doesn't account for
    maintainers submitting patches to files which they themselves
    maintain.  This is fine for situations where there are are multiple
    maintainers, but not for situations where there is only one
    maintainer.
    
    Add an explicit "Check-in policy" section to the MAINTAINERS document
    to serve as the canonical reference for the check-in policy.  Move
    paragraphs not explicitly related to nesting into it.
    
    While here, "promote" the "The meaning of nesting" section title.
    
    DISCUSSION
    
    This seems to be a change from people's understanding of the current
    policy.  Most people's understanding of the current policy seems to be:
    
    1.  In order to get a change to a given file committed, it must have
    an Ack or Review from at least one *maintainer* of that file other
    than the submitter.
    
    2. In the case where a file has only one maintainer, it must have an
    Ack or Review from a "nested" maintainer.
    
    I.e., if I submitted something to x86/mm, it would require an Ack from
    Jan or Andy, or (in exceptional circumstances) The Rest; but an Ack from
    (say) Roger or Juergen wouldn't suffice.
    
    Let's call this the "maintainer-ack" approach (because it must have an
    ack or r-b from a maintainer to be checked in), and the proposal in
    this patch the "maintainer-approval" (since SoB from a maintainer
    indicates approval).
    
    The core issue I have with "maintainer-ack" is that it makes the
    maintainer less privileged with regard to writing code than
    non-maintainers.  If component X has maintainers A and B, then a
    non-maintainer can have code checked in if reviewed either by A or B.
    If A or B wants code checked in, they have to wait for exactly one
    person to review it.
    
    In fact, if B is quite busy, the easiest way for A really to get their
    code checked in might be to hand it to a non-maintainer N, and ask N
    to submit it as their own.  Then A can Ack the patches and check them
    in.
    
    The current system, therefore, either sets up a perverse incentive (if
    you think the behavior described above is unacceptable) or unnecessary
    bureaucracy (if you think it's acceptable).  Either way I think we
    should set up our system to avoid it.
    
    Other variations on "maintainer-ack" have been proposed:
    
    - Allow maintainer's patches to go in with an R-b from "designated
      reviewers"
    
    - Allow maintainer's patches to go in with an Ack from more general
      maintainer
    
    Both fundamentally make it harder for maintainers to get their code in
    and/or reviewed effectively than non-maintainers, setting up the
    perverse incentive / unnecessary bureaucracy.
    
    Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 MAINTAINERS | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0bfd1cb6ba..46424c1833 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -104,7 +104,63 @@ Descriptions of section entries:
           xen-maintainers-<version format number of this file>
 
 
-The meaning of nesting:
+       Check-in policy
+       ===============
+
+In order for a patch to be checked in, in general, several conditions
+must be met:
+
+1. In order to get a change to a given file committed, it must have
+   the approval of at least one maintainer of that file.
+
+   A patch of course needs Acks from the maintainers of each file that
+   it changes; so a patch which changes xen/arch/x86/traps.c,
+   xen/arch/x86/mm/p2m.c, and xen/arch/x86/mm/shadow/multi.c would
+   require an Ack from each of the three sets of maintainers.
+
+   See below for rules on nested maintainership.
+
+2. It must have appropriate approval from someone other than the
+   submitter.  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
+     not available; see the secton on nested maintainership)
+
+  b. A Reviewed-by by anyone of suitable stature in the community
+
+3. Sufficient time must have been given for anyone to respond.  This
+   depends in large part upon the urgency and nature of the patch.
+   For a straightforward uncontroversial patch, a day or two may be
+   sufficient; for a controversial patch, a week or two may be better.
+
+4. There must be no "open" objections.
+
+In a case where one person submits a patch and a maintainer gives an
+Ack, the Ack stands in for both the approval requirement (#1) and the
+Acked-by-non-submitter requirement (#2).
+
+In a case where a maintainer themselves submits a patch, the
+Signed-off-by meets the approval requirement (#1); so a Review
+from anyone in the community suffices for requirement #2.
+
+Before a maintainer checks in their own patch with another community
+member's R-b but no co-maintainer Ack, it is especially important to
+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.
+
+Maintainers may choose to override non-maintainer objections in the
+case that consensus can't be reached.
+
+As always, no policy can cover all possible situations.  In
+exceptional circumstances, committers may commit a patch in absence of
+one or more of the above requirements, if they are reasonably
+confident that the other maintainers will approve of their decision in
+retrospect.
+
+       The meaning of nesting
+       ======================
 
 Many maintainership areas are "nested": for example, there are entries
 for xen/arch/x86 as well as xen/arch/x86/mm, and even
@@ -118,11 +174,6 @@ the Ack of the xen/arch/x86/mm/shadow maintainer for that 
part of the
 patch, but would not require the Ack of the xen/arch/x86 maintainer or
 the xen/arch/x86/mm maintainer.
 
-(A patch of course needs acks from the maintainers of each file that
-it changes; so a patch which changes xen/arch/x86/traps.c,
-xen/arch/x86/mm/p2m.c, and xen/arch/x86/mm/shadow/multi.c would
-require an Ack from each of the three sets of maintainers.)
-
 2. In unusual circumstances, a more general maintainer's Ack can stand
 in for or even overrule a specific maintainer's Ack.  Unusual
 circumstances might include:
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog