Skip to content

chore(codeowners): widen *, /src/, /tests/, /packages/ to five owners - #5844

Open
sonichi wants to merge 2 commits into
mainfrom
chore/codeowners-widen
Open

sonichi wants to merge 2 commits into
mainfrom
chore/codeowners-widen

Conversation

@sonichi

@sonichi sonichi commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

Priority

Level: high
Reason: approved PRs are waiting on a code-owner review that only one or two people can give; this widens who can give it.

Closes

N/A — owner ask ("add more codeowners"), no issue.

Summary

What: add @keweichen @liususan091219 @marklysze to the four broad .github/CODEOWNERS lines — *, /src/, /tests/ and /packages/. Existing owners stay; the skill-specific lines are untouched. A two-line comment above * records the reason and points here for the numbers. A new /.github/ @sonichi @qingyun-wu line near the end of the file (last match wins) keeps CODEOWNERS itself and the CI workflows with the two original owners, so none of the three new owners can approve a change to ownership or to workflows.

Why: the main ruleset requires a code-owner review and forbids self-approval. With only @sonichi @qingyun-wu on the broad lines, PRs sit fully approved by two reviewers who are not code owners and still cannot merge — a single available code owner has become the merge bottleneck, and the file's own GOVERNANCE note already calls a singleton owner "a hard block".

Evidence (review load, measured): reviews submitted since 2026-09-24 on the 50 most recently updated PRs (gh api 'repos/sonichi/sutando/pulls?state=all&per_page=50&sort=updated&direction=desc', then pulls/<n>/reviews per PR, counted per user.login; 457 reviews total on those PRs, 328 of them in the window):

login reviews (30d) distinct PRs
liususan091219 105 50
keweichen 98 14
john-the-dev 51 13
qingyun-wu 46 19
sonichi 9 5
yixuan-ag2 7 3
marklysze 6 4
bassilkhilo-ag2 5 4
akag2 1 1

So the numbers support Susan and Kewei as the two highest-volume non-owner reviewers. They do not support "bulk of reviews" for Mark (6 reviews; @john-the-dev has 51 and is not added). Mark is included on the owner's instruction, and the file comment says "see PR body" rather than claiming a count for him. Reviewer call: if Mark should be dropped from these lines, or John added, say so and I will push the change.

Reviewers, look first at: the four changed owner lines and the two comment lines above * (last-match-wins ordering is unchanged, so the more specific /src/..., /tests/... lines below still override these).

Feature/documentation consistency: N/A — no doc under docs/ lists the owner set; CONTRIBUTING.md/REVIEW.md describe the ruleset, not the names.

Note on approving this PR: the author's own approval never counts toward the code-owner requirement, so a code owner other than the author (@qingyun-wu, or after merge any of the added names) must approve.

Checklist

  • Confirmed no other open PR closes the same issue — gh pr list --search codeowners: chore(codeowners): add @thinkall as Windows co-owner #5498 (adds @thinkall Windows lines) is the only open CODEOWNERS PR; it adds new lines only and does not touch these four
  • Git author email matches my CLA-signed email (4250911+sonichi@users.noreply.github.com)
  • Single concern per PR
  • Confirmed the state exists on origin/main (broad lines carry two owners at 7940f25ff)
  • Test added — N/A, config-only; validated by GitHub's CODEOWNERS parser below
  • Before/after evidence pasted below
  • Every claim in this PR body is verifiable from the diff or the pasted output
  • Live path? N/A
  • Stacked PR? N/A
  • Scanned added lines for host-specific hardcoded paths (scripts/review-checks.sh below)
  • CI workflow? N/A
  • Workspace resolution? N/A

Before / after evidence

GitHub's own CODEOWNERS parser, main (before) and this branch (after) — no new errors introduced:

$ gh api repos/sonichi/sutando/codeowners/errors
{"errors":[]}
$ gh api 'repos/sonichi/sutando/codeowners/errors?ref=chore/codeowners-widen'
{"errors":[]}   # re-checked at 43e68e7b with the /.github/ line

Owner lines before → after:

-* @sonichi @qingyun-wu
+* @sonichi @qingyun-wu @keweichen @liususan091219 @marklysze
-/src/ @sonichi @qingyun-wu
+/src/ @sonichi @qingyun-wu @keweichen @liususan091219 @marklysze
-/tests/ @sonichi @qingyun-wu @liususan091219
+/tests/ @sonichi @qingyun-wu @liususan091219 @keweichen @marklysze
-/packages/                 @qingyun-wu @sonichi
+/packages/                 @qingyun-wu @sonichi @keweichen @liususan091219 @marklysze
+/.github/                  @sonichi @qingyun-wu

Test plan

$ bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD)
prose-cap: no in-scope files (exts .py); nothing was scanned
review-checks: PARTIAL (hardcoded-paths + root-artifacts clean; prose-cap had no in-scope files)

PARTIAL only because the diff has no .py; the two scanners that apply are clean. All three added logins already appear on other lines of the file on main (which parses with zero errors), so they satisfy the write-access requirement.

🤖 Generated with Claude Code

A single code owner has become the merge bottleneck: PRs sit fully
approved by two reviewers who are not code owners, and the main ruleset
still requires a code-owner review, so they wait on the same one or two
people. Widen the four broad lines to the people already carrying the
review load; the skill-specific lines are unchanged.

Measured over the 50 most recently updated PRs, reviews submitted since
2026-09-24: liususan091219 105, keweichen 98, marklysze 6 (Mark is
owner-assigned; his count does not by itself justify the line).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@sonichi
sonichi requested a review from qingyun-wu as a code owner October 24, 2026 09:12
The widening above lets the three new owners approve code changes; it
must not let them approve a change to ownership itself or to the CI
workflows. A `/.github/` line near the end wins over `*` (last match
wins) and keeps those with @sonichi and @qingyun-wu.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at 43e68e7. Mechanics check out: GitHub's CODEOWNERS parser reports no errors at this head, all five logins are collaborators (sonichi admin, the other four write), last-match order is preserved with /.github/ @sonichi @qingyun-wu last, and the main ruleset still requires one code-owner review with no dismiss-on-push. Two things the owner should decide knowingly, so I am leaving the approval to Qingyun rather than giving it under the login: (1) one approval from any of the five now suffices for most of the repo, including the scripts that implement the required checks (scripts/coverage-gate.sh, scripts/review-checks.sh, lint-*.sh, REVIEW.md), and several of these accounts post agent-written reviews; the body's "none of the new owners can approve workflow changes" is true of the YAML only. (2) #5498 appends a windows-runtime.yml … @thinkall line that must stay below the new /.github/ line or thinkall loses it. Nit: the comment above /src/ still says "both dominant authors".

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 43e68e7 (agent-assisted review, Sutando for Qingyun). Verified the widened *, /src/, /tests/ and /packages/ owner lists parse with no CODEOWNERS errors at this head, and that the new last-line /.github/ rule (last match wins) keeps CI and ownership files with the original two owners.

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 43e68e7. This is ready to merge.

Disclosure: this login (liususan091219) is one of the three owners this PR adds, so read this approval as concurrence, not an independent gate. The code-owner requirement is already met by @qingyun-wu's approval at this head.

What I verified:

  • gh api 'repos/sonichi/sutando/codeowners/errors?ref=43e68e7b…' returns {"errors":[]}.
  • Main's CODEOWNERS hasn't changed since the merge-base (the branch is 120 commits behind but merges cleanly: git merge-tree is clean). In the merged result /.github/ @sonichi @qingyun-wu is still the last line, so last-match-wins keeps CODEOWNERS and the workflows with the two original owners.
  • The more specific /src/…, /tests/… and /skills/… lines below the widened ones still override them, because the ordering is unchanged.
  • CI is all green, license/cla is success (1 context), and there are 0 unresolved threads. The change is config only, so no live test applies.

Not blocking, carried over from the earlier comment at this head: the /.github/ carve-out covers YAML and ownership only. The scripts behind the required checks (scripts/coverage-gate.sh, scripts/review-checks.sh, lint-*.sh, REVIEW.md) can now be approved by any of the five. Also, the comment above /src/ still says "both dominant authors". And #5498 is still open: its windows-runtime.yml … @thinkall line has to land BELOW the new /.github/ line, or the last-match rule takes that line away from @thinkall.

— Sutando (susans-MacBook-Pro)

This branch has not been deployed

No deployments
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.

3 participants