diff --git a/Developer/MAINTAINING.md b/Developer/MAINTAINING.md index cf0f031..23532ae 100644 --- a/Developer/MAINTAINING.md +++ b/Developer/MAINTAINING.md @@ -58,11 +58,18 @@ Tag reviewers by skillset: ### 4.1 Picking the reviewer -CODEOWNERS requests reviewers when you open the PR. Check that at least one requested reviewer isn't you or a co-author, and add one by hand if not. The reviewer either takes the PR or replies within **2 working days** with "can't take this — @umeradl". Silence past 2 working days escalates to Umer. There's no fixed rotation. We'll revisit that if third-party PRs stall. +CODEOWNERS requests reviewers when you open the PR. Check that at least one requested reviewer isn't you or a co-author, and add one by hand if not. The reviewer either takes the PR or replies within **2 working days** with "can't take this — @umeradl". Silence past 2 working days escalates to Umer. + +**Tie-breaker for first reviews.** There's no fixed rotation. When more than one maintainer in the [reviewer pool](#3-reviewer-pool) is eligible and has the skillset for a PR, the first review goes to whoever has done the fewest reviews that week. Review count doesn't capture effort, so also weigh each person's outstanding reviews and how complex they are, and their availability. The tie-breaker only chooses among eligible reviewers: the skillset table in [No. 3](#3-reviewer-pool) still decides who is eligible, and recusal or escalation is still the right answer when a PR is outside your depth. + +**The 2-working-day window covers every review request**, not just the first response: the `foundry/src/**` second sign-off ([No. 4.2](#42-review-depth)) and re-reviews after the author pushes new commits (which dismiss stale approvals, [No. 2](#2-what-github-enforces)) count too. It's a window to respond or escalate in, not a deadline to finish the review: + +- If a review needs more time, the reviewer says so on the PR within the window and states what's left to check. +- If a second sign-off has waited more than 2 working days, the peer reviewer pings @umeradl and @abrahamnash on the PR. For PRs from other authors, Umer or Abraham can instead route it to a security reviewer ([No. 4.2](#42-review-depth)) who isn't the PR's author or its peer reviewer. ### 4.2 Review depth -- **Any PR touching `foundry/src/**`** (contract logic, storage layout, upgrade paths) needs the peer's review **plus a second sign-off** from @umeradl or @abrahamnash, or from a security reviewer they route it to. Mechanism changes also need the [mechanism sign-off](#mechanism-sign-off) below. +- **Any PR touching `foundry/src/**`** (contract logic, storage layout, upgrade paths) needs the peer's review **plus a second sign-off** from @umeradl or @abrahamnash. When the author is @robertocarlous or @Abidoyesimze, the second sign-off is always @umeradl or @abrahamnash. For other authors, Umer or Abraham may route it to a security reviewer who isn't the PR's author or its peer reviewer. Mechanism changes also need the [mechanism sign-off](#mechanism-sign-off) below. - Docs-only, test-only, Python and tooling PRs need one peer. **What counts as evidence.** Required checks that are green on the PR count as evidence for anything CI covers. The merge queue re-runs them against the latest `develop` before anything lands. The reviewer cites the CI run and doesn't re-run it. **Local reproduction is required only for what CI doesn't cover:** @@ -96,7 +103,7 @@ Templates: [`.claude/skills/din-pr-review/templates/`](../.claude/skills/din-pr- ### 4.4 Merge paths -- **(A) GitHub-side merge commit — the default.** The reviewer approves, CI is green, and the reviewer clicks **Merge when ready** to add the PR to the merge queue. The queue re-runs `CI OK` on the PR merged onto the latest `develop` and lands it with a merge commit. If that run fails, the queue drops the PR and leaves `develop` untouched; fix it on the PR branch and queue it again. If the reviewer wants something changed first, they push the fix onto the PR branch (or open a small follow-up PR). The last-push rule then sends the final approval to someone else. +- **(A) GitHub-side merge commit — the default.** The reviewer approves, CI is green, and the reviewer clicks **Merge when ready** to add the PR to the merge queue. **The approver queues by default:** whoever gives the final required approval queues the PR once every required check and sign-off is in, unless their review says otherwise. If they can't queue it, or aren't eligible to merge it under [No. 1](#1-the-rule) (for example, they co-authored it), they hand the step to a named eligible maintainer in the review, so no approved PR sits without an owner. When the last thing in is a `foundry/src/**` second sign-off from Umer or Abraham, Umer may queue it himself, and otherwise the peer reviewer does. The queue re-runs `CI OK` on the PR merged onto the latest `develop` and lands it with a merge commit. If that run fails, the queue drops the PR and leaves `develop` untouched; fix it on the PR branch and queue it again. If the reviewer wants something changed first, they push the fix onto the PR branch (or open a small follow-up PR). The last-push rule then sends the final approval to someone else. - **(B) Local merge plus follow-up fix commit — exceptional, Umer only.** For PRs that need adapting against a `develop` that has moved (renames, ABI regeneration, docs drift). It needs the admin bypass. **The admin bypass** is for emergencies and path B only. Each use gets a one-line reason in [Discussion 170](https://github.com/InfiniteZeroFoundation/DevNet/discussions/170), followed by a normal reviewed PR where possible.