From nobody Thu Apr 18 05:09:20 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail(p=none dis=none) header.from=citrix.com ARC-Seal: i=1; a=rsa-sha256; t=1578927962; cv=none; d=zohomail.com; s=zohoarc; b=FjjKeQQU61YZWUPYH9W03YPmqFpKHE6dl7zMIzi56uDYOkBFVOI3KRYWKN8F0S7yq5Fj1ktdl38/IZCvVMQ7jBZOfLtzYX0XOIfeGZptZqt4smI3pkivGeYy4n3Arn1J7/kOzwPbXgDkj/AG9a319j48446Asye2ptFxavczLCU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1578927962; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=7wFzaV04Bs6D87jDdkjDv8R7JpicC7y+dyT7yuHw5+o=; b=X/Iim6boq0Yv5lU69NDX5BbV3AgSh6+2eoDVVVNoWnf+K75bEI5Cmw2X7CRWWx4ZXb8qZXmd7qFMyYyHZYLLhYos0S4AGeUrZ0pRrwbrW83leolJrPus57aFgaCyuoPWhOgi5yXz8ZLO4kjv8o1y/NimRJ9hsNpkrXXsz//KUCg= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1578927962074375.9931061166411; Mon, 13 Jan 2020 07:06:02 -0800 (PST) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ir1H4-0005s4-Kl; Mon, 13 Jan 2020 15:05:02 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ir1H4-0005rz-2h for xen-devel@lists.xenproject.org; Mon, 13 Jan 2020 15:05:02 +0000 Received: from esa1.hc3370-68.iphmx.com (unknown [216.71.145.142]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 11461e70-3616-11ea-827f-12813bfff9fa; Mon, 13 Jan 2020 15:05:00 +0000 (UTC) X-Inumbo-ID: 11461e70-3616-11ea-827f-12813bfff9fa DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1578927900; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=vZpSXFkHEMyBm2Lao36vm+ZMvZvvbr7hoBdfTUBQRL0=; b=YTVvWAvk0fxIe7oqc/YhouoNO7T22qpAhhotmDu6W2hyuDxRy11IbUVK RV4ogqaAeU9bskmmQ05bH+NlSRLlgagP0EI7t6qfJ8rtca8JWOHjtzNcj +u01NBDzgRQij1rFvpG4H+DaOEY+o/gqJcVl+aahuDfzNN6pr/cJ8Vyg3 E=; Authentication-Results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=george.dunlap@citrix.com; spf=Pass smtp.mailfrom=George.Dunlap@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of george.dunlap@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="George.Dunlap@citrix.com"; x-sender="george.dunlap@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa1.hc3370-68.iphmx.com: domain of George.Dunlap@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="George.Dunlap@citrix.com"; x-sender="George.Dunlap@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="George.Dunlap@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: kzvcVWa7rp4DVp5rHBsvBlgsAA1F+pSJeEe43JWhXYThM+tSfCfhzQEx6uYa7fqqj0/RrxpY1B aYmp5wJLpdGrDzbtu9qGLeAhxf7lzyg29Tn/jbNXubR+fdwVfbJGHKUWcz7FijETuSydjj1BvV B94h2qrsO4F55uzyXMF/anvqKXDkaSkBGKwU5K8SLY7yQY3lLontuP2wQzli48ylE+Tog2KU5o 0qboJZ3nXanIYvhpbusRcJNCC102+X93PY4xW5jmIW2lK5f9NQuCYis1tDf0D6EgyUSUJyPCkw LvQ= X-SBRS: 2.7 X-MesageID: 11001217 X-Ironport-Server: esa1.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.69,429,1571716800"; d="scan'208";a="11001217" From: George Dunlap To: Date: Mon, 13 Jan 2020 15:04:55 +0000 Message-ID: <20200113150455.400733-1-george.dunlap@citrix.com> X-Mailer: git-send-email 2.24.1 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v4] MAINTAINERS: Add explicit check-in policy section X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Lars Kurth , Stefano Stabellini , Julien Grall , Wei Liu , Konrad Wilk , Andrew Cooper , Tim Deegan , George Dunlap , Jan Beulich , Ian Jackson Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) 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 Acked-by: Jan Beulich --- v4: - Try to reword the "someone other than the submitter" section to be more clear. - Make a distinction between "waiting sufficient time for anyone to respond" and "waiting a sufficient time for a co-maintainer to respond" v3: - Be more specific about kind of tag (R-b or Ack) is required from what kind of person ('normal' person or maintainer) - Separate "reasonable amount of time for anyone to object in general" from "reasonable amount of time / notification for checking in without a maintainer ack". v2: - Modify "sufficient time" to "sufficient time and/or warning". - Add a comment explicitly stating that there are exceptions. - Move some of the alternate proposals into the changelog itself CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper CC: Jan Beulich CC: Tim Deegan CC: Konrad Wilk CC: Stefano Stabellini CC: Julien Grall CC: Lars Kurth This is a follow-up to the discussion in `[PATCH for-4.12] passthrough/vtd: Drop the "workaround_bios_bug" logic entirely`, specifical= ly Message-ID: <5C9CF25A020000780022291B@prv1-mh.provo.novell.com> Another approach would be to say that in the case of multiple maintainers, the maintainers themselves can decide to mandate each other'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. Checks that maintainers themselves have agreed on will produce neither perverse incentives, nor be considered "unnecessary". --- MAINTAINERS | 63 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index d5bd83073c..79f665b5b2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -104,7 +104,63 @@ Descriptions of section entries: xen-maintainers- =20 =20 -The meaning of nesting: + Check-in policy + =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +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 + =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =20 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 t= hat part of the patch, but would not require the Ack of the xen/arch/x86 maintainer or the xen/arch/x86/mm maintainer. =20 -(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: --=20 2.24.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel