Conversation
4d70e7f to
69516ff
Compare
There was a problem hiding this comment.
Pull request overview
Adds PR governance automation to enforce/encourage linking PRs to issues and to provide prioritization context (sprint/milestone) back to contributors, plus documentation/spellcheck updates to support the workflow.
Changes:
- Add a new
pr-governanceGitHub Actions workflow that checks for linked/closing issues, comments guidance, and optionally checks Project sprint + milestone. - Update
cli/azd/CONTRIBUTING.mdto require PRs be linked to an issue and explain prioritization expectations. - Update cspell GitHub handle aliases to include additional maintainers/handles referenced by the new guidance/comments.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
.github/workflows/pr-governance.yml |
New workflow to enforce issue linkage and post sprint/milestone prioritization context (with comment dedupe/cleanup). |
cli/azd/CONTRIBUTING.md |
Adds contributor guidance to link PRs to issues and explains priority review expectations. |
.vscode/cspell-github-user-aliases.txt |
Adds/normalizes GitHub handle aliases used in docs/workflow comments to avoid cspell noise. |
69516ff to
45c03f7
Compare
Adds a GitHub Actions workflow that runs on PRs targeting main: - Checks for a linked GitHub issue (hard fail if missing) - Posts a prioritization comment based on the issue's milestone - Skips dependabot PRs and PRs with 'skip-governance' label - Updates comment in place on subsequent runs (no duplicates) Also updates CONTRIBUTING.md with linked issue guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
45c03f7 to
0dfdbc2
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
There was a problem hiding this comment.
Issues to address:
- pr-governance-priority-check.js:87 - sprint match accepts any sprint assignment, not just the current one
- pr-governance.yml - no concurrency group; parallel runs can create duplicate bot comments
- pr-governance-priority-check.js:30 -
graphqlWithTokendoesn't check HTTP status before parsing JSON - pr-governance-priority-check.js:4 -
JSON.parseon env var has no error handling - pr-governance-priority-check.js:70 - GraphQL query uses string interpolation instead of variables
kristenwomack
left a comment
There was a problem hiding this comment.
Code Review — PR #7428: Add PR governance workflow
Findings from Copilot testing
1. Misleading comment when milestone lookup fails — priority-check.js:100–135
When REST calls to fetch milestones fail, the script falls through and posts "The linked issue isn't in the current milestone yet" — telling the contributor their work is deprioritized when the script simply couldn't determine status. If all lookups failed, skip commenting or post a "couldn't determine" message.
2. Unprotected GraphQL call in issue-check — issue-check.js:37–43
No try/catch on github.graphql() and no null guards on the result chain. A rate-limit or transient error gives the contributor a raw stack trace instead of the helpful bot comment.
3. Comment listing calls unprotected in both scripts — issue-check.js:47–52, priority-check.js:131–136
github.paginate() is outside any try/catch, but the subsequent create/update is wrapped. If listing fails, the script crashes before posting any guidance.
4. Shared BOT_MARKER causes cross-script comment clobbering — both scripts
Both scripts use <!-- pr-governance-priority -->. When a contributor edits their PR to add an issue link, priority-check will overwrite or delete the "Linked Issue Required" comment originally posted by issue-check. Use distinct markers or extract a shared constant with documented single-comment-per-PR semantics.
5. Duplicated comment-upsert logic (DRY) — both scripts
Identical paginate → find marker → create-or-update pattern in both files. Extract to a shared .github/scripts/lib/bot-comment.js utility before this becomes copy-paste tax on future governance checks.
Product feedback
The three-tier escalation (in-sprint → in-milestone → not-yet-prioritized) gives contributors useful signal about where they stand. The skip-governance label and dependabot exemptions are sensible, and the CONTRIBUTING.md update sets expectations upfront. The comment copy is clear and actionable. One gap worth addressing: closingIssuesReferences only catches closing keywords (Fixes #123) and sidebar links — a contributor who writes "Related to #123" won't be detected. It would help to call that out in the bot comment so they understand why their link wasn't recognized and what to do about it.
- Fix sprint match to check current sprint only (not past/future) - Add concurrency group to prevent duplicate comments - Add response.ok check before parsing JSON - Defensive parsing of ISSUE_NUMBERS env var - Use parseInt for GraphQL issue number interpolation - Skip comment when all lookups fail (avoid misleading message) - Wrap GraphQL and paginate calls in try/catch - Use separate BOT_MARKER per script (issue vs priority) - Skip draft PRs, add ready_for_review trigger Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
All feedback addressed — sprint match fixed, concurrency group added, error handling hardened, draft PR skip added.
jongio
left a comment
There was a problem hiding this comment.
Previous feedback looks well-addressed. A few remaining items:
- pr-governance-priority-check.js:46 - magic number
182repeated in two queries - pr-governance.yml:20 - no timeout-minutes on the job
- pr-governance.yml:6 - missing
reopenedevent trigger
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Fixes #7427
Adds a workflow that checks PRs for linked issues and posts prioritization context.
skip-governancelabeled PRsAlso updates CONTRIBUTING.md with linked issue guidance.
Tested scenarios
Tested on fork: rajeshkamal5050/pull/2