ci: name @MetaMask/extension-platform owner of five skill domains - #156
Draft
MajorLift wants to merge 2 commits into
Draft
ci: name @MetaMask/extension-platform owner of five skill domains#156MajorLift wants to merge 2 commits into
@MetaMask/extension-platform owner of five skill domains#156MajorLift wants to merge 2 commits into
Conversation
`observability`, `platform`, `security`, `stability` and `typescript` fall to the `*` catch-all today, so a change to any of them is owned by both platform teams jointly and by neither in particular. The paths are added ahead of the directories, which arrive in the PRs adding each domain. No workflow validates CODEOWNERS paths, so an entry for a path that does not exist yet is inert rather than failing. One entry per domain keeps this out of the five PRs that would otherwise each edit this file and conflict.
A domain holds the cross-client skill as well as the overlays, so naming one platform team owner of a whole domain claims the shared half too. Both platform teams co-own each domain, matching `coding`, `general`, `performance` and `pr-workflow`. Ownership of a single client's material is expressed where that material actually is: the `repos/<client>.md` overlay. Those rules follow the domain block so they take precedence, and they name each of the five domains rather than globbing, because `/domains/**/repos/metamask-extension.md` would move 11 files away from `@MetaMask/qa`, `@MetaMask/swaps-engineers`, `@MetaMask/design-system-engineers` and `@MetaMask/perps`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Five skill domains have no owner.
observability,platform,security,stabilityandtypescriptfall through to the*catch-all, which names both platform teams, so a change to any of them is owned jointly and by nobody in particular. The domains that already have entries do not work that way:assetsgoes to@MetaMask/metamask-assets,perpsto@MetaMask/perps,testingto@MetaMask/qa.Ownership has to be expressed at the right granularity, because a domain is not single-client. It holds the cross-client
skill.mdas well as the per-client overlays, so naming one platform team owner of a whole domain claims the shared half too.Overview
Two rules per domain rather than one.
The domain is co-owned by
@MetaMask/extension-platformand@MetaMask/mobile-platform, matchingcoding,general,performanceandpr-workflow.A client's overlay is owned by that client's platform team, expressed where the client-specific material actually lives:
The overlay rules follow the domain block, because CODEOWNERS resolves last-match-wins and they have to beat the domain rule to apply at all.
They name each of the five domains rather than globbing the domain segment. A blanket
/domains/**/repos/metamask-extension.mdreads better and would move 11 files away from teams that own them today: 7 from@MetaMask/qa, 2 from@MetaMask/swaps-engineers, and one each from@MetaMask/design-system-engineersand@MetaMask/perps. The wildcard stays on the skill-name segment, where it is safe.The domain block is also re-sorted alphabetically so the five land in place rather than at the end.
Showcase
The paths are added before the directories exist, since each domain arrives in its own PR. Nothing reads CODEOWNERS in CI: greping all five workflows and
lint-skill-entry.mjsforcodeownersreturns zero, against 14 hits forskillinlint-skill-entry.ymlas a control. An entry for a path that does not exist yet is inert.Validated with GitHub's own checker rather than by eye,
GET /repos/MetaMask/skills/codeowners/errors:mainEvery error on both refs is
Unknown owner, andmainalready has 30. That is team visibility to the querying token, not a defect in the file. The delta of 20 is exactly the new lines: 5 domains x 2 teams, plus 10 overlay rules x 1 team. No error of any other kind appears, which is what confirms the*patterns parse; an invalid pattern is reported as one.Doing this as one PR rather than five is deliberate. Five PRs each editing this block conflict with each other, which two open ones already do.