Skip to content

Remove CODEOWNERS and fix maintainer-approval in merge queue#4931

Merged
simonfaltum merged 8 commits intomainfrom
simonfaltum/remove-codeowners
Apr 13, 2026
Merged

Remove CODEOWNERS and fix maintainer-approval in merge queue#4931
simonfaltum merged 8 commits intomainfrom
simonfaltum/remove-codeowners

Conversation

@simonfaltum
Copy link
Copy Markdown
Member

@simonfaltum simonfaltum commented Apr 11, 2026

Why

  1. Approval enforcement is now handled by the maintainer-approval GitHub Action and the .github/OWNERS file. GitHub's built-in CODEOWNERS is no longer needed.

  2. The maintainer-approval workflow only triggers on pull_request_target and pull_request_review events. It never runs for merge_group events, so the status is never set on the merge queue commit. This blocks the merge queue indefinitely.

  3. maintainer-approval uses commit statuses (createCommitStatus), which are not clickable in the GitHub checks UI. Check runs (checks.create) show a details page with logs and output.

Changes

CODEOWNERS removal: Removes .github/CODEOWNERS. The .github/OWNERS file already contains the same ownership rules, and the maintainer-approval workflow reads from OWNERS.

Merge queue fix: Adds merge_group trigger to the maintainer-approval workflow with an auto-approve job. PRs are already approved before entering the merge queue, so the job sets a passing check on the merge queue commit. This follows the same pattern as the Integration Tests auto-approve in push.yml.

Commit status to check run: Migrates all createCommitStatus calls to checks.create in both the JS script and the YAML workflow. Permissions updated from statuses: write to checks: write. The check job is also guarded with github.event_name != 'merge_group' to prevent it from running (and failing) on merge queue events.

Test plan

  • Verified OWNERS file covers all paths from CODEOWNERS
  • maintainer-approval is a required status check on main
  • All 20 tests in maintainer-approval.test.js pass with the check run migration
  • Verify merge queue succeeds after merging this change

Approval is now enforced through the maintainer-approval GitHub Action
(required status check) and the OWNERS file, rather than GitHub's
built-in CODEOWNERS mechanism. This gives us more flexible per-path
approval logic while keeping the same ownership rules.

Co-authored-by: Isaac
Adds support for "team:<name>" aliases in the OWNERS file. Teams are
defined in .github/OWNERTEAMS and expanded by the owners.js parser.
This avoids repeating long member lists on every line.

Defines team:platform (PAE) and team:bundle, and updates OWNERS to
use them.

Co-authored-by: Isaac
…rom template owners

Extract shared line-parsing logic into readDataLines helper. Replace
existsSync+readFileSync with try/catch ENOENT. Remove simonfaltum
from /libs/template/ owners.

Co-authored-by: Isaac
# Use "team:<name>" in OWNERS to reference a team defined here.
# Format: team:<name> @member1 @member2 ...

team:bundle @andrewnester @anton-107 @denik @pietern @shreyas-goenka
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add @janniklasrose as well?

@simonfaltum simonfaltum added this pull request to the merge queue Apr 13, 2026
@simonfaltum simonfaltum removed this pull request from the merge queue due to a manual request Apr 13, 2026
The maintainer-approval workflow only triggers on pull_request_target
and pull_request_review events, so the status is never set on the
merge queue commit. This adds a merge_group trigger that auto-approves
the status, following the same pattern as the Integration Tests check.

Co-authored-by: Isaac
@simonfaltum simonfaltum enabled auto-merge April 13, 2026 09:09
Commit statuses are not clickable in the GitHub UI. Check runs show
a details page with logs and output summary. This makes it easier
to see why a PR is pending or who approved it.

Co-authored-by: Isaac
@simonfaltum simonfaltum changed the title Remove CODEOWNERS in favor of OWNERS file Remove CODEOWNERS and fix maintainer-approval in merge queue Apr 13, 2026
@simonfaltum simonfaltum disabled auto-merge April 13, 2026 09:32
@simonfaltum simonfaltum merged commit 8bc2b1a into main Apr 13, 2026
22 checks passed
@simonfaltum simonfaltum deleted the simonfaltum/remove-codeowners branch April 13, 2026 09:32
simonfaltum added a commit that referenced this pull request Apr 13, 2026
The previous change (PR #4931) switched maintainer-approval from
commit statuses to check runs. However, check runs with status
"in_progress" are treated by GitHub as "still running" and don't
block the required status check, so all PRs could merge without
approval.

Fix: for approved PRs, create a success check run (green, clickable).
For pending PRs, create nothing. The required status check stays as
"Expected" in GitHub (yellow dot), which blocks the merge until
approval is granted.

Co-authored-by: Isaac
github-merge-queue bot pushed a commit that referenced this pull request Apr 13, 2026
## Why

PR #4931 switched `maintainer-approval` from commit statuses
(`createCommitStatus`) to check runs (`checks.create`) so the check is
clickable in the GitHub UI. The pending state used `status:
"in_progress"`, which GitHub treats as "still running" rather than
"blocking". This meant all PRs could merge without maintainer approval.

## Changes

Removes the three `checks.create` calls for pending states (wildcard
files, uncovered groups, no approval). When no check run or status
exists for `maintainer-approval` on a SHA, GitHub shows the required
check as "Expected" (yellow dot) and blocks the merge. Approved PRs
still get a success check run (green, clickable).

The result:
- **No approval**: yellow dot, merge blocked, reviewer info in PR
comment
- **Approved**: green checkmark, clickable, shows who approved
- **Merge queue**: green checkmark, auto-approved (unchanged)

## Test plan

- [x] All 20 tests in `maintainer-approval.test.js` pass
- [ ] Verify on a subsequent PR (after merge) that `maintainer-approval`
shows yellow "Expected" without approval, then turns green after
approval

Note: the workflow uses `pull_request_target`, so it runs from main.
This PR cannot test itself.
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