Azure / Azure/azure-rest-api-specs
Consolidate the three independent protected-labels.yml parsers into a shared resolver
- Dominant language
- TypeSpec
- Stars
- 3.1k
- Forks
- 5.9k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 444
Description
## Consolidate the three independent `protected-labels.yml` parsers
### Problem
`.github/protected-labels.yml` is currently read and parsed by three separate
modules, each rolling its own loader and login-extraction logic:
- `.github/workflows/src/protected-labels/check-label.js` -> `loadConfig` +
`resolveAuthorizedUsers`. Not exported. Validation-heavy, plane-aware.
- `.github/workflows/src/package-name-approval/approvers.js` -> `loadApproversConfig`.
Exported, but shaped specifically for `package-name-*` namespace entries.
- `.github/workflows/src/data-plane-review/assign-reviewers.js` -> `loadConfig` +
`extractLogins` + `loadAuthorizedSigners`.
None of these is reusable by the others: `check-label.js` does not export its
internals, and `approvers.js` returns a namespace-specific nested shape. The
result is three copies of "read the yaml, flatten logins, merge global-approvers,
handle the flat-vs-plane-aware entry shapes," each with slightly different
handling.
### Proposal
Extract a single shared module (for example
`.github/workflows/src/protected-labels/config.js`) that exposes:
- a `loadProtectedLabelsConfig(path?)` reader, and
- an `authorizedLoginsFor(config, label, { plane?, includeGlobals? })` resolver
that handles both the flat (`Label: [users]`) and plane-aware
(`Label: { management-plane: [...], data-plane: [...] }`) entry shapes and
the case-insensitive login comparison in one place.
Then update `check-label.js`, `approvers.js`, and `assign-reviewers.js` to call
the shared helper instead of their private copies.
### Scope / notes
- Pure refactor, no behavior change intended. Existing unit tests for all three
consumers must stay green.
- `check-label.js` intentionally throws on malformed config; the shared reader
should preserve that strict validation so enforcement never runs on a bad file.
- Low priority. Deliberately kept out of the assignment-only PR #45313 to avoid
dragging `check-label.js` back into that change set.
Contributor guide
Assessment
This issue has not been assessed yet.