Azure / Azure/azure-rest-api-specs

Proposal: data-plane review label consolidation

Open
#45,437 1 comment 0 reactions 1 assignee Claimed by @samvaity View on GitHub
Dominant language
TypeSpec
Stars
3.1k
Forks
5.9k
Avg merge
2d 22h
Merged PRs (30d)
444

Description

Scope: data-plane stewardship review labels only. Not ARM labels. Not merging stewardship
review with SDK/namespace review.

## Context

We are wiring up data-plane review now (reviewer assignment, board view, review agent). The labels these depend on use three different naming styles, and some of them are barely used or never used. Pick one naming scheme and drop the dead labels before we build more on top of them.

Styles in use today:
- PascalCase: `SuppressionReviewRequired`, `BreakingChangeReviewRequired`
- kebab: `data-plane`, `new-api-version`, `package-name--approved`
- hybrid: `APIStewardshipBoard-ReviewRequested`, `APIStewardship-ChangesRequested`

## What the flow actually needs

The data-plane review flow needs exactly two labels:

| Label | Applied by | Purpose |
|---|---|---|
| `data-plane-review-requested` | intake (auto) | signals a review is needed; triggers reviewer assignment; the board/queue filters on it |
| `data-plane-review-signoff` | reviewer pool (protected) | reviewer sign-off; clears the merge gate |

Everything else is either a duplicate of these or a state that GitHub already tracks natively (a "changes requested" PR review).

## Today's labels and what we do with each

| Label today | PRs ever labeled | Used by automation? | Plan |
|---|---|---|---|
| `APIStewardshipBoard-SignedOff` | 405 | yes (merge gate + who may apply it) | rename to `data-plane-review-signoff` |
| `APIStewardshipBoard-ReviewRequested` | 60 | yes (assignment trigger) | rename to `data-plane-review-requested` |
| `data-plane-api-review-needed` | 0 | yes (review agent trigger) | replace with `data-plane-review-requested` (one trigger, not two) |
| `offline-review-requested` | 37 | no | drop; fold into `data-plane-review-requested` (redundant "wants review" marker; reviews are offline now) |
| `APIStewardship-ChangesRequested` | 9 | no | drop; use the native GitHub "changes requested" review state |
| `APIStewardshipBoard-InDesign` | 1 | no | drop (effectively unused) |

## The protected sign-off label

`data-plane-review-signoff` (today `APIStewardshipBoard-SignedOff`) is a protected label: only an authorized group of reviewers can apply it, and anyone else's attempt is removed automatically. This is what stops a service team from self-signing-off to clear the merge
gate. That same authorized group is the reviewer pool, managed as the GitHub team [`@Azure/azure-data-plane-api-reviewers`](https://github.com/orgs/Azure/teams/azure-data-plane-api-reviewers/members), so the roster changes independently of code. See
[PROTECTED-LABELS.md](https://github.com/Azure/azure-rest-api-specs/blob/main/.github/workflows/src/protected-labels/PROTECTED-LABELS.md) for how the authorized group is defined and enforced.

## Naming scheme

`data-plane-review-`, kebab-case. An ARM-plane version of this review exists, so naming by plane keeps it obvious and lets ARM mirror it later as `arm-review-*`. Names built from plane + state survive a board rename (unlike `APIStewardshipBoard-*`).

## Rollout

- Now (safe, no live labels touched): create the new kebab labels; drop the dead ones (`InDesign`, `ChangesRequested`, `offline-review-requested`); point the review agent at `data-plane-review-requested`.
- At the [Project 196](https://github.com/orgs/Azure/projects/196/views/19) board retirement: rename `SignedOff` and `ReviewRequested` (code + applied name in one PR), then delete the old names.

## The workflow this supports (today vs where we are going)

The labels are the signals on one review loop: detect -> request -> assign -> sign off ->
merge. Where each piece stands:

| Step | Today | Where we are going |
|---|---|---|
| Detect + label | `labelling.js` auto-applies `data-plane` and `new-api-version`. Review-requested is applied by hand. | Auto-apply `data-plane-review-requested` when a PR is `data-plane` + `new-api-version`. It is a new review-state label added on top; it does not replace `data-plane`/`new-api-version`, which stay as classification. |
| Assign reviewer | Manual, or an early script that requests the whole pool. | Native GitHub team review-request picks one reviewer from the pool team. Filter the board on review-requested-from-team, not assignee. |
| Board view | No shared view. | Shared queue filtered on `data-plane-review-requested`. |
| Sign off + merge gate | Reviewer applies the protected sign-off label; gate clears merge. | Same, renamed to `data-plane-review-signoff`. |
| Review agent | Prototype triggered by `data-plane-api-review-needed` (0 real runs, on hold). | Same agent triggered by `data-plane-review-requested`, so agent and humans share one signal. |

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.