Changelog verification#2580
Conversation
anannya03
requested review from
KarishmaGhiya,
g2vinay,
jairmyree,
msalaman,
saikoumudi,
vukelich and
wbreza
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds CI enforcement for changelog hygiene by requiring either a changelog entry in the PR diff or an explicit skip-changelog PR label, including GitHub authentication in PR pipelines to enable label checks.
Changes:
- Introduces
Test-ChangelogEntry.ps1to detect changelog entry files and/or validateskip-changelogvia GitHub REST API. - Hooks the new validation into
Analyze-Code.ps1and fails analysis when missing both entry and label. - Updates the analyze pipeline job to authenticate to GitHub for PR repos.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| eng/scripts/Test-ChangelogEntry.ps1 | Adds the changelog/label validation script using git diff + GitHub API label lookup |
| eng/scripts/Analyze-Code.ps1 | Executes changelog validation and turns failures into analysis errors |
| eng/pipelines/templates/jobs/analyze.yml | Adds GitHub login step for PR repos so label checks can authenticate |
|
|
||
| Push-Location $RepoRoot | ||
| try { | ||
| $changedFiles = git diff --name-only --diff-filter=A "origin/main...HEAD" 2>&1 |
| @@ -0,0 +1,56 @@ | |||
| #!/bin/env pwsh | |||
| Write-Host "PR #$prNumber labels: $($prLabels -join ', ')" | ||
| } | ||
| catch { | ||
| Write-Warning "Failed to fetch PR labels from GitHub API: $_" |
|
|
||
| Push-Location $RepoRoot | ||
| try { | ||
| $changedFiles = git diff --name-only --diff-filter=A "origin/main...HEAD" 2>&1 |
| exit 0 | ||
| } | ||
|
|
||
| Write-Host "No changelog entry and no 'skip-changelog' label. If changelog entry is not required, add the 'skip-changelog' label to the PR to bypass this check. Or add a changelog entry TO the PR." |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Two bugs that'll prevent this from working correctly:
-
parameters.Repositorydoesn't exist inanalyze.yml- the condition on line 12 will always evaluate to false, so the GitHub login step never runs.common.ymlcalls this template without passing aRepositoryparameter. -
Regex misses
.yamlfiles inTest-ChangelogEntry.ps1- the pattern only matches.ymlbut this repo has both.ymland.yamlchangelog entries.
Also: 8 commits with debug messages ('trial 1', 'trial 2', etc.) - please squash before merge.
|
|
||
| - template: /eng/pipelines/templates/steps/install-dotnet.yml | ||
|
|
||
| - ${{ if endsWith(parameters.Repository, '-pr') }}: |
There was a problem hiding this comment.
parameters.Repository isn't declared in this template and common.yml (line 79) calls it without passing one - so this always evaluates as endsWith('', '-pr') which is always false. The login step never runs.
Other templates in this repo use variables['Build.Repository.Name'] for this check (see create-apireview.yml:40, eng-common-workflow-enforcer.yml:38).
| - ${{ if endsWith(parameters.Repository, '-pr') }}: | |
| - ${{ if endsWith(variables['Build.Repository.Name'], '-pr') }}: |
| Write-Error "Failed to determine changed files with 'git diff --name-only --diff-filter=AM $diffRange'. $gitError" | ||
| exit 1 | ||
| } | ||
| $hasChangelog = $changedFiles | Where-Object { $_ -match 'changelog-entries/.*\.yml$' -or $_ -match 'CHANGELOG\.md'} |
There was a problem hiding this comment.
This only matches .yml but the repo has both .yml and .yaml files in changelog-entries/ directories (checked - there are 5 of each). You'll miss valid entries with the .yaml extension.
| $hasChangelog = $changedFiles | Where-Object { $_ -match 'changelog-entries/.*\.yml$' -or $_ -match 'CHANGELOG\.md'} | |
| $hasChangelog = $changedFiles | Where-Object { $_ -match 'changelog-entries/.*\.ya?ml$' -or $_ -match 'CHANGELOG\.md'} |
github-project-automation
Bot
moved this from Untriaged
to In Progress
in Azure MCP Server
anannya03
force-pushed
the
changelog_verify
branch
from
1d24b9f to
9141365
Compare
What does this PR do?
Adds changelog verification so PRs must either include a changelog entry or carry the
skip-changeloglabel.This change updates the Build pipeline to authenticate to GitHub for PR repos, invokes a new
Test-ChangelogEntry.ps1validation step fromAnalyze-Code.ps1, and adds the new script that checks changed files for changelog entries and falls back to validating the PR label through the GitHub REST API.GitHub issue number?
#2495
Pre-merge Checklist
servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationREADME.mdchanges running the script./eng/scripts/Process-PackageReadMe.ps1. See Package READMEToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.jsonbreaking-changelabelservers/Azure.Mcp.Server/docs/azmcp-commands.md./eng/scripts/Update-AzCommandsMetadata.ps1to update tool metadata inazmcp-commands.md(required for CI)servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline