Skip to content

Assign per-language architects when a review is ready - #10167

Merged
Sameeksha Vaity (samvaity) merged 4 commits into
Azure:mainfrom
samvaity:arch-board-assign-reviewers
Jul 27, 2026
Merged

Sameeksha Vaity (samvaity) merged 4 commits into
Azure:mainfrom
samvaity:arch-board-assign-reviewers

Conversation

@samvaity

@samvaity Sameeksha Vaity (samvaity) commented Jul 22, 2026 •

Copy link
Copy Markdown
Member

What

When triage marks a board-review issue ready-for-review, automatically assign one architect per selected language so they are notified through GitHub's native notifications instead of having to watch the project board.

Why

The project board is visibility-only today - there is no assignment or notification. Reviews can sit unnoticed when architects are busy. Assignment routes each review to a real person through their existing GitHub notification preferences, with nothing custom to maintain.

How

  • New assign-reviewers.js: resolves one architect per selected language from the shared api-review-approvers.yml roster (round-robin by issue number), reusing the approval-check.js helpers. Management-plane issues also consider the management-plane approvers.
  • triage.js: on ready-for-review, assigns via addAssignees (this is the notification), then lists the assigned handles inside the existing "All materials verified" comment - a single comment, no extra ping.
  • Idempotent: re-edits of an already-assigned issue do not re-assign or re-notify.
  • Documents the assignment step and the C++/Rust language labels in ARCH-BOARD-REVIEW-PROCESS.md.

Design note

Inspired by the config-driven, plane-aware pattern in Azure/azure-rest-api-specs#44664, but reuses the existing roster and helpers rather than duplicating a reviewer list - single source of truth, no drift.

Copilot AI review requested due to automatic review settings July 22, 2026 00:40
@samvaity
Sameeksha Vaity (samvaity) requested a review from a team as a code owner July 22, 2026 00:40
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

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.

Pull request overview

Automatically assigns language architects when board-review issues become ready.

Changes:

  • Adds deterministic, plane-aware reviewer assignment.
  • Integrates assignments into triage comments and tests.
  • Documents reviewer assignment and supported labels.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
assign-reviewers.js Resolves and assigns architects.
triage.js Triggers assignment and displays handles.
assign-reviewers.test.js Tests assignment behavior.
triage.test.js Tests triage integration.
ARCH-BOARD-REVIEW-PROCESS.md Documents the workflow.

When triage marks a board-review issue ready-for-review, resolve one
architect per selected language from the shared api-review-approvers.yml
roster (round-robin by issue number) and assign them. Assignment fires
GitHub's native notifications so architects are reached through their own
preferences. The resolved handles are shown in the existing "materials
verified" comment (single comment, no extra ping). Idempotent: re-edits
do not re-assign.

- Add rust data-plane approver (heaths) to api-review-approvers.yml.
- Surface selected languages with no configured architect as "unassigned"
  in the triage comment instead of silently omitting them.
- Document the assignment step and C++/Rust language labels in
  ARCH-BOARD-REVIEW-PROCESS.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f922dad1-6173-4c65-b4bb-03f056572963
Address remaining PR review comments:

- r3626637027: render assigned handles as inline code instead of @login in
  the success comment so it does not fire a redundant mention ping (the
  addAssignees call is the notification).
- r3626637091: reconcile stale assignments. When an edit changes the selected
  languages, architects the automation previously assigned but no longer
  resolved are removed via removeAssignees. Only roster members are removed,
  so manually added (non-roster) assignees are always preserved.

Add getAllApprovers helper to approval-check.js and cover reconciliation and
manual-assignee preservation in tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f922dad1-6173-4c65-b4bb-03f056572963
- Wrap assignReviewers in try/catch in triage so a failed assignment
  (invalid handle, permissions, API hiccup) still posts the success
  comment instead of leaving a ready-for-review issue with no feedback.
- Add a reason field to the assignReviewers skipped result to
  distinguish 'already-assigned' from 'no-reviewers-resolved'.
- Strengthen the no-comment test to assert createComment is not called
  (mock now includes createComment) rather than checking mock shape.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f922dad1-6173-4c65-b4bb-03f056572963
Per reviewer feedback:
- Replace the round-robin picker with assigning every configured architect
  for each selected language. Simpler and no state; if it gets noisy for
  languages with multiple architects we can add distribution later.
- When the assignment API call fails, the 'materials verified' comment now
  includes a visible note asking for manual assignment (with a link to the
  approvers file) instead of only logging a warning, so the issue is never
  left silently unassigned.
- Add a link to api-review-approvers.yml in the unassigned-language note too.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f922dad1-6173-4c65-b4bb-03f056572963
@samvaity
Sameeksha Vaity (samvaity) merged commit 85bb4bb into Azure:main Jul 27, 2026
5 checks passed
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