All of lore.kernel.org
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@citrix.com>
To: xen-devel@lists.xenproject.org
Cc: Lars Kurth <lars.kurth@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>,
	Ian Jackson <ian.jackson@citrix.com>
Subject: [PATCH] MAINTAINERS: Add explicit check-in policy section
Date: Wed, 8 May 2019 12:39:47 +0100	[thread overview]
Message-ID: <20190508113947.11920-1-george.dunlap@citrix.com> (raw)

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
maintaniers, but not for situations where there is only one
maintianer.

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.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Konrad Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: Lars Kurth <lars.kurth@citrix.com>

This is a follow-up to the discussion in `[PATCH for-4.12]
passthrough/vtd: Drop the "workaround_bios_bug" logic entirely`, specifically
Message-ID: <5C9CF25A020000780022291B@prv1-mh.provo.novell.com>

This encodes my understanding of the policy, and what I think is the
best one.

A second approach would 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.

A third approach would be to say that in the case of multiple
maintainers, the maintainers themselves can decide to mandate the
other maintainer's Ack.  For instance, Dario and I could agree that we
don't need each others' ack for changes to the scheduler, but Andy and
Jan could agree that they do need each other's Ack for changes to the
x86 code.
---
 MAINTAINERS | 46 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e43388ddb0..65ba35f02d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -99,7 +99,46 @@ Descriptions of section entries:
 	   One regex pattern per line.  Multiple K: lines acceptable.
 
 
-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 an Acked-by or a Reviewed-by from someone other than
+   the submitter.
+
+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 is
+   sufficient; for a controversial patch, longer (maybe a week) would
+   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 requriment (#1); so an Ack or Review
+from anyone in the community suffices for requirement #2.
+
+Maintainers may choose to override non-maintainer objections in the
+case that consensus can't be reached.
+
+       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
@@ -113,11 +152,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:
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

WARNING: multiple messages have this Message-ID (diff)
From: George Dunlap <george.dunlap@citrix.com>
To: <xen-devel@lists.xenproject.org>
Cc: Lars Kurth <lars.kurth@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>,
	Ian Jackson <ian.jackson@citrix.com>
Subject: [Xen-devel] [PATCH] MAINTAINERS: Add explicit check-in policy section
Date: Wed, 8 May 2019 12:39:47 +0100	[thread overview]
Message-ID: <20190508113947.11920-1-george.dunlap@citrix.com> (raw)
Message-ID: <20190508113947.FSj0dxelcrFoeCxZ-oxsQxzzfYt4BuN2vON8iUHYaYo@z> (raw)

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
maintaniers, but not for situations where there is only one
maintianer.

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.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Konrad Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: Lars Kurth <lars.kurth@citrix.com>

This is a follow-up to the discussion in `[PATCH for-4.12]
passthrough/vtd: Drop the "workaround_bios_bug" logic entirely`, specifically
Message-ID: <5C9CF25A020000780022291B@prv1-mh.provo.novell.com>

This encodes my understanding of the policy, and what I think is the
best one.

A second approach would 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.

A third approach would be to say that in the case of multiple
maintainers, the maintainers themselves can decide to mandate the
other maintainer's Ack.  For instance, Dario and I could agree that we
don't need each others' ack for changes to the scheduler, but Andy and
Jan could agree that they do need each other's Ack for changes to the
x86 code.
---
 MAINTAINERS | 46 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e43388ddb0..65ba35f02d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -99,7 +99,46 @@ Descriptions of section entries:
 	   One regex pattern per line.  Multiple K: lines acceptable.
 
 
-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 an Acked-by or a Reviewed-by from someone other than
+   the submitter.
+
+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 is
+   sufficient; for a controversial patch, longer (maybe a week) would
+   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 requriment (#1); so an Ack or Review
+from anyone in the community suffices for requirement #2.
+
+Maintainers may choose to override non-maintainer objections in the
+case that consensus can't be reached.
+
+       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
@@ -113,11 +152,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:
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

             reply	other threads:[~2019-05-08 11:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-08 11:39 George Dunlap [this message]
2019-05-08 11:39 ` [Xen-devel] [PATCH] MAINTAINERS: Add explicit check-in policy section George Dunlap
2019-05-08 11:59 ` Juergen Gross
2019-05-08 11:59   ` [Xen-devel] " Juergen Gross
2019-05-08 13:45   ` George Dunlap
2019-05-08 13:45     ` [Xen-devel] " George Dunlap
2019-05-09 11:05     ` Ian Jackson
2019-05-09 11:05       ` [Xen-devel] " Ian Jackson
2019-05-09 11:36       ` Jan Beulich
2019-05-09 11:36         ` [Xen-devel] " Jan Beulich
2019-05-08 13:49 ` Jan Beulich
2019-05-08 13:49   ` [Xen-devel] " Jan Beulich
2019-05-08 15:32 ` Stefano Stabellini
2019-05-08 15:32   ` [Xen-devel] " Stefano Stabellini
2019-05-09 11:16 ` Ian Jackson
2019-05-09 11:16   ` [Xen-devel] " Ian Jackson
2019-05-09 11:45   ` George Dunlap
2019-05-09 11:45     ` [Xen-devel] " George Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190508113947.11920-1-george.dunlap@citrix.com \
    --to=george.dunlap@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=lars.kurth@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.