Skip to content

docs: replace the fast path with the enforced review baseline - #3288

Merged
M4n5ter merged 1 commit into
mainfrom
docs/contributing-fast-path-approval
Aug 20, 2026
Merged

docs: replace the fast path with the enforced review baseline#3288
M4n5ter merged 1 commit into
mainfrom
docs/contributing-fast-path-approval

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Branch protection from #3262 makes main require an approving review and a passing test check, and dismisses stale approvals on every new commit. CONTRIBUTING.md still described a fast path that merges "without independent human review", which a committer can no longer do.

Redefining the fast path as "the baseline alone" does not rescue it. The extra scrutiny in that section applies only to protected areas, and not touching a protected area was already a fast-path precondition, so the two scopes never overlap: a named fast path would exempt nothing and only add a comment obligation, and the maintainer sign-off it claimed to skip was never written down anywhere.

So this states the rule directly instead. The section now gives the baseline every pull request clears, names a maintainer as the one who decides whether a change is material and whether the review it received is enough, and closes with "for everything else the baseline is enough" — which answers which changes take the light route, without keeping a concept that no longer carries content. Naming the classifier preserves what the old "a maintainer makes the final determination" actually did. The merge-time comment does not survive: it existed to leave a trace for merges that had no approval, and every merge now has one.

Two corrections land in the same section:

  • The baseline says branch protection enforces the mechanics, and that independent human judgment is policy GitHub cannot verify. An approval from someone other than the author is not by itself proof of an independent human: GitHub cannot tell a human from an agent-operated account, and require_last_push_approval is not set, so a collaborator who pushed to the branch can approve it. Separately, enforcement_level is non_admins, so branch protection does not bind admins at all.
  • The Chinese text is realigned so 重大 distributes across the whole protected list, matching the English, rather than modifying only the first item.

Separately, the public-decision rule moves from future to present tense. dev@maka.apache.org exists and carries active threads, so project-level decisions belong there now rather than "once an ASF development list is available".

.coderabbit.yaml and .github/skills/code-review/SKILL.md told automated reviewers not to make a fast-path determination. With the concept gone, both now say only that automated review is not authorization to merge. Their neighbouring lines about independent human review are still accurate and are left alone.

Refs #3262

Verification

  • The approval requirement is live: every open pull request based on main reports reviewDecision: REVIEW_REQUIRED, while feat(desktop): manage remote runtime host services #3282, based on a feature branch, reports nothing. gh api repos/apache/maka/branches/main --jq '.protected' is true with test in .protection.required_status_checks.contexts.
  • /branches/main/protection needs admin rights and returns 404 for a committer, so required_approving_review_count and dismiss_stale_reviews are read from .asf.yaml on main. The reviewDecision evidence above confirms the requirement is in effect; the exact count is not independently verified.
  • dev@maka.apache.org is live — the list archive reports threads in 2026-08.
  • Documentation and review-bot configuration only. Biome does not format Markdown or this YAML, so lint, format, build, typecheck and the test suites are unaffected.
  • Both tables of contents are updated for the renamed section; no other file links to the old anchor.

Review focus

This records a decision already taken in public rather than making a new one. #3262 put branch protection on main, and that is what left the fast path with nothing to exempt; this pull request writes the consequence into the document. Nothing here grants a permission that #3262 did not already settle, or drops a check that still functions: the baseline paragraph describes what branch protection enforces, "a maintainer decides whether a change is material" carries over the old "a maintainer makes the final determination", the merge-time comment lost its only subject with the fast path, and "for everything else the baseline is enough" is stricter than the text it replaces. So it is not raised on dev@maka.apache.org separately.

What would be a new decision is putting a light-weight route back under a different definition — self-merge once the baseline is met, say. That is deliberately not done here, and it belongs on the list if anyone wants it.

Reviewers should push back on that framing if they read the change as more than bookkeeping.

The branch is two commits behind main. Those commits touch only apps/desktop and packages/ui, strict is false in .asf.yaml, so it is not rebased.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code drafted the replacement wording in both languages, the commit message, and this description. Two adversarial review passes by the same tool shaped the result: the first found that an earlier draft left the fast path as an empty concept, the second found that dropping it also dropped the named decider for whether a change is material. Both findings were checked by hand against the repository and the GitHub API before being acted on, and neither counts as independent human review. The commit carries a Generated-by: Claude Code trailer. The human contributor of record reviews the final diff and owns the decision to submit.

Checklist

  • Tests cover the change and fail without it — n/a, no code
  • Lint, format, typecheck and the affected suites pass locally — n/a, no code

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@Astro-Han
Astro-Han force-pushed the docs/contributing-fast-path-approval branch from 38dea5b to 21a59b0 Compare August 20, 2026 06:17
@Astro-Han Astro-Han changed the title docs: align the fast path with enforced branch protection docs: replace the fast path with the enforced review baseline Aug 20, 2026
Branch protection landed in .asf.yaml (#3262) and now requires an approving review and a passing `test` check on every pull request to main, with stale approvals dismissed on each new commit. Every open pull request based on main reports reviewDecision REVIEW_REQUIRED; #3282, based on a feature branch, reports nothing.

That leaves the fast path with nothing to exempt. CONTRIBUTING defined it as merging without independent human review, which a committer can no longer do. Redefining it as "the baseline alone" does not rescue it: the extra scrutiny in this section applies only to protected areas, and not touching a protected area was already a fast-path precondition, so the two scopes never overlap. A named fast path would exempt nothing while adding a comment obligation, and the maintainer sign-off it claimed to skip was never written down anywhere.

State the rule directly instead. The section gives the baseline every pull request clears, names a maintainer as the one who decides whether a change is material and whether the review it received is enough, and closes with "for everything else the baseline is enough" — which answers which changes take the light route without keeping a concept that no longer carries content. Naming the classifier keeps what the old "a maintainer makes the final determination" actually did: settle whether a change needs more than the mechanics. The merge-time comment does not survive; it existed to leave a trace for merges that had no approval, and every merge now has one.

Two corrections in the same section. The baseline says branch protection enforces the mechanics and that independent human judgment is policy GitHub cannot verify, because an approval from someone other than the author is not by itself proof of an independent human. The Chinese text is realigned so 重大 distributes across the whole protected list, matching the English, instead of modifying only the first item.

The public-decision rule moves from future to present tense. dev@maka.apache.org exists and carries active threads, so project-level decisions belong there now rather than "once an ASF development list is available".

.coderabbit.yaml and .github/skills/code-review/SKILL.md told automated reviewers not to make a fast-path determination. With the concept gone, both now say only that automated review is not authorization to merge. Their neighbouring lines about independent human review remain accurate and are left alone.

Generated-by: Claude Code
@Astro-Han
Astro-Han force-pushed the docs/contributing-fast-path-approval branch from 21a59b0 to b612c9f Compare August 20, 2026 06:36
@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 06:56
@Astro-Han
Astro-Han requested a review from M4n5ter August 20, 2026 07:01

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@M4n5ter
M4n5ter merged commit 6095203 into main Aug 20, 2026
1 check passed
@M4n5ter
M4n5ter deleted the docs/contributing-fast-path-approval branch August 20, 2026 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants