From 5f0f8cbd1c1465b7897bd07d77240d8bee65e726 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 19 Oct 2026 01:01:54 -0700 Subject: [PATCH 1/2] MAINTAINERS_GUIDE: clarify and relax PR merge rules Many pull requests stay open for weeks waiting for a second review, which slows down development considerably. Spell out the merge rules, and relax them for maintainers: a pull request authored by a maintainer can be merged with a single LGTM from another maintainer, provided it has been open for review for at least 7 days (14 days for large changes, i.e. more than 100 lines), so that other maintainers had a chance to object. Also, allow routine pull requests (opened by Dependabot, or only changing documentation and/or CI configuration) to be merged with a single LGTM. Neither of the above applies to governance changes, i.e. changes to the project rules (such as this document) or their enforcement, which always require two LGTMs. While at it, clarify that: - an LGTM is a pull request approval; - a maintainer requesting changes blocks the merge. This is the same as done in runc (see opencontainers/runc#5512). Signed-off-by: Kir Kolyshkin --- MAINTAINERS_GUIDE.md | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS_GUIDE.md b/MAINTAINERS_GUIDE.md index 8e96917..642581e 100644 --- a/MAINTAINERS_GUIDE.md +++ b/MAINTAINERS_GUIDE.md @@ -64,11 +64,37 @@ made through a pull request. All decisions are pull requests, and the relevant maintainers make decisions by accepting or refusing the pull request. Review and acceptance -by anyone is denoted by adding a comment in the pull request: `LGTM`. +by anyone is denoted by approving the pull request (an `LGTM`). However, only currently listed `MAINTAINERS` are counted towards the required -two LGTMs. In addition, if a maintainer has created a pull request, they cannot -count toward the two LGTM rule (to ensure equal amounts of review for every pull -request, no matter who wrote it). +LGTMs, and the pull request author's own LGTM is never counted. + +A pull request can be merged once it has: + +* two LGTMs from maintainers; or +* one LGTM from a maintainer, if it is a routine change; or +* one LGTM from another maintainer, if the author is a maintainer, the pull + request is not a governance change, and it has been open for review (that + is, not a draft) for at least 7 days, or 14 days if it is a large change + (more than 100 lines added and removed). This gives other maintainers a + chance to review and object. + +A pull request is a governance change if it changes the rules of the project, +or how they are enforced. That is, it changes `MAINTAINERS`, `CODEOWNERS`, +a top-level `*.md` file other than `README.md` (such as this document), or the +`review-policy` workflow. A governance change always requires two LGTMs. + +A pull request is a routine change if it is not a governance change, and it is +either: + +* opened by Dependabot; or +* only changes documentation (`*.md` files) and/or CI configuration (files + under `.github/`). + +In all cases, a maintainer requesting changes blocks the merge until the +request is resolved (the reviewer approves, or the review is dismissed). + +These rules are enforced by the +[`review-policy`](.github/workflows/review-policy.yml) check. Overall the maintainer system works because of mutual respect. The maintainers trust one another to act in the best interests of the project. From 620f380501a2009530c013e13d22fe229e94b4a4 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 19 Oct 2026 01:02:05 -0700 Subject: [PATCH 2/2] ci: add review-policy check Add a check enforcing the pull request merge rules from MAINTAINERS_GUIDE.md, to be used as a required status check (in addition to the GitHub's required number of approvals, which can not depend on who the author is, and also counts approvals from anyone having write access, not just from current maintainers). The check is a pull_request_review workflow with a read-only token, which evaluates the current state of the pull request using the GitHub API. No other triggers are used, since branch protection requires the latest run for each event type to pass, so a failed run triggered by, say, a push would keep blocking the merge after a review fixes it. As a result, the check needs a review (a comment-only one is fine) to be run after a push, and it needs to be re-run manually once a review period is over. Note that since the workflow runs from the pull request, a pull request can change it. This is the price for not using privileged triggers (pull_request_target or workflow_run) and write permissions. To mitigate, the workflow is a governance file (so changing it requires two LGTMs), and GitHub's own required approval stays as a backstop. The check itself is the runc one (see opencontainers/runc#5512), used as a reusable workflow (made so in opencontainers/runc#5573), pinned to a commit. Dependabot is told not to bump it, since a bump may change the rules, so it should be a deliberate (and governance) change. Signed-off-by: Kir Kolyshkin --- .github/dependabot.yml | 3 +++ .github/workflows/review-policy.yml | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 .github/workflows/review-policy.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d770afb..5f7d17e 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -10,3 +10,6 @@ updates: interval: "daily" cooldown: default-days: 7 + ignore: + # Used by review-policy.yml. Bump manually, as it may change the rules. + - dependency-name: "opencontainers/runc" diff --git a/.github/workflows/review-policy.yml b/.github/workflows/review-policy.yml new file mode 100644 index 0000000..d06779e --- /dev/null +++ b/.github/workflows/review-policy.yml @@ -0,0 +1,23 @@ +# Enforce the pull request merge rules described in MAINTAINERS_GUIDE.md +# ("Who decides what?"). This job is a required status check. +# +# The actual check is in the runc repository; see the comments there for +# details. Always pin it to a commit SHA. It is not bumped by dependabot +# (see dependabot.yml), since a bump may change the merge rules. +# +# As with any pull_request_review workflow, this file is run from the pull +# request, so a pull request can change it. Therefore it is a governance file, +# and changes to it must be reviewed carefully. +name: review-policy +on: + pull_request_review: + types: [submitted, edited, dismissed] + +permissions: {} + +jobs: + review-policy: + permissions: + contents: read + pull-requests: read + uses: opencontainers/runc/.github/workflows/review-policy.yml@c9232a3698424b71706c4d802a4395c3734cbcaf