Conversation
- Remove obsolete release-phase-1 and release-phase-3 workflows. - Update release-phase-2 workflow to include version validation and license updates. - Enhance build verification documentation for clarity. - Add a new release comment template for license updates.
- Changed "npm – Test Clean" to "npm – Clean" - Updated "Set version environment variable" to "Set Version Environment Variable" - Renamed "npm – Install" to "npm – Install Dependencies"
- Added default shell configuration to PowerShell in build, release-phase-1, and release-phase-2 workflows. - Updated scripts to use PowerShell syntax for better consistency and readability.
Extract the inline PowerShell change detection logic, duplicated across two jobs in the build workflow, into a shared support script `Test-Changes.ps1`. Update step names and IDs for consistency.
- Removed individual version update steps from release-phase-1.yml. - Added Update-Version.ps1 script to handle version updates for multiple files. - Simplified version number management and ensured consistency across files.
- Extract file content update logic into a reusable function. - Define shared replacement patterns for versioning. - Update multiple files with new version information using the new function.
- Update patch version to maintain consistency. - Adjust logic to compute and set the correct patch version for releases. - Ensure self-update of workflow file reflects the current patch version.
- Removed unnecessary Raw parameter and logic. - Streamlined content retrieval and update process.
- Changed variable name from `$Version` to `$version` for consistency. - Improves readability and maintains coding standards. docs: remove outdated note on release signing - Deleted note regarding releases prior to v1.7.12 for clarity.
- Introduce Update-Licenses.ps1 for license notice handling - Implement Test-NoticePresent and Remove-Notices functions - Add Test-NoticeResult.ps1 for PR comment posting based on license checks - Update release-phase-1.yml to integrate license truncation - Create Push-SignedCommit.ps1 for committing license updates
- Replace Write-Host with direct output redirection for PATCH and VERSION in release workflows. - Update Test-Changes.ps1 and Test-NoticeResult.ps1 for consistent variable naming and output logic.
- Introduced a new parameter `forceLicenseGeneration` to control license generation. - Updated workflows to utilize the new parameter for conditional license updates. - Added error and warning message files for better feedback during license generation.
- Updated scripts to use Write-Output for better output handling. - Improved consistency across workflow scripts. - Removed unnecessary lines from license generation warning file.
- Implement ShouldProcess support in Remove-Notice function for safer execution. - Update Update-FileContent function to include ShouldProcess for file updates. - Simplify log content formatting in Test-NoticeResult.ps1.
- Retrieve branch HEAD OID for commit mutation - Create signed commit via GraphQL API - Handle errors from GraphQL response
- Changed error message file from .txt to .md format for consistency. - Added new license generation error message. - Added new license generation warning message.
- Add logic to delete previous license comments on LICENSE.txt. - Include HEAD SHA in comments for better traceability. - Append build task output to error and warning comments.
- Implement Phase 1 for release preparation with version validation and environment setup. - Create Phase 2 for release publishing, including artifact signing and GitHub release creation. - Add support for version tracking with a trigger file.
- Change workflow names from "Phase 1" and "Phase 2" to "Prepare" and "Publish" respectively. - Update file paths in workflows to reflect new naming conventions. - Modify verification documentation to reference updated workflow names.
- Create a new workflow for release preparation. - Automate version number validation and updates. - Include steps for license truncation and dependency updates. - Implement Git operations for branch creation and pushing changes. - Generate a pull request with autogenerated release notes.
- Updated workflow name from "Release – Prepare" to "Release – Phase 1" - Adjusted file path reference in Update-Version script
PR Metrics✔ Thanks for keeping your pull request small.
Metrics computed by PR Metrics. Add it to your Azure DevOps and GitHub PRs! |
There was a problem hiding this comment.
Pull request overview
This PR consolidates the release automation from a 3-phase process to a streamlined 2-phase approach. Phase 1 creates a release PR with version updates, license truncation, and dependency updates. The new Publish workflow (formerly Phase 3) builds, signs, attests, and publishes the GitHub release when triggered by changes to the release-publish-trigger.txt file. Phase 2 has been eliminated entirely. The changes also introduce reusable PowerShell scripts for version management and license automation, and add automated license generation support to the Azure DevOps PR pipeline.
Changes:
- Consolidated 3-phase release workflow into 2 phases by eliminating Phase 2 and merging its functionality into Phase 1
- Extracted inline YAML logic into reusable PowerShell scripts for version updates, license management, and GitHub API interactions
- Added automated license generation capability to Azure DevOps PR builds with Component Governance integration
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/release-publish.yml |
Renamed from release-phase-3.yml; builds, signs, attests, and publishes releases when trigger file changes |
.github/workflows/release-phase-1.yml |
Refactored to use new PowerShell scripts; creates release PR with version/license/dependency updates |
.github/workflows/release-phase-2.yml |
Deleted; functionality merged into Phase 1 |
.github/workflows/build.yml |
Refactored to use Test-Changes.ps1 script; added default PowerShell shell |
.github/workflows/support/Update-Version.ps1 |
New script to update version numbers across multiple files |
.github/workflows/support/Update-Licenses.ps1 |
New script to truncate/check LICENSE.txt for notice generation |
.github/workflows/support/Test-NoticeResult.ps1 |
New script to check notice task results and post PR comments |
.github/workflows/support/Test-Changes.ps1 |
New script to detect git changes for conditional workflow steps |
.github/workflows/support/Push-SignedCommit.ps1 |
New script to create signed commits via GitHub GraphQL API |
.github/workflows/support/release-comment.md |
New PR comment template with license update instructions |
.github/workflows/support/license-generation-error.md |
New template for license generation error comments |
.github/workflows/support/license-generation-warning.md |
New template for license generation warning comments |
.github/workflows/support/release-publish-trigger.txt |
New trigger file for Publish workflow (replaces release-phase-3-trigger.txt) |
.github/workflows/support/release-phase-3-trigger.txt |
Deleted; replaced by release-publish-trigger.txt |
.github/azure-devops/template.yml |
Added automated license generation with notice@0 task integration |
.github/azure-devops/pr.yml |
Added forceLicenseGeneration parameter for PR builds |
.github/actions/release-phase-1-internal/action.yml |
Deleted; logic moved to workflow and PowerShell scripts |
.github/actions/git-setup-and-push/action.yml |
Deleted; functionality replaced by inline workflow steps |
docs/verification.md |
Updated workflow references from release-phase-3.yml to release-publish.yml |
.editorconfig |
Updated trigger file pattern to match new release-publish-trigger.txt |
| branchName = $branch | ||
| } | ||
| message = @{ | ||
| headline = 'chore: update licence notices' |
There was a problem hiding this comment.
Inconsistent spelling: "licence" should be "license" to match the American English spelling used throughout the rest of the codebase.
| $reviewComments = Invoke-RestMethod -Uri "$repoApi/pulls/$prNumber/comments?per_page=100" -Headers $commentHeaders | ||
| foreach ($comment in $reviewComments) | ||
| { |
There was a problem hiding this comment.
Potential bug: The GitHub API call uses 'per_page=100' but doesn't handle pagination. If a PR has more than 100 review comments, older comments matching the error/warning templates won't be deleted. Consider implementing pagination to iterate through all pages of comments, or using a higher page limit if GitHub's API allows it.
| $reviewComments = Invoke-RestMethod -Uri "$repoApi/pulls/$prNumber/comments?per_page=100" -Headers $commentHeaders | |
| foreach ($comment in $reviewComments) | |
| { | |
| # Fetch all review comments with pagination (GitHub API returns max 100 per page). | |
| $allReviewComments = @() | |
| $page = 1 | |
| while ($true) | |
| { | |
| $pageComments = Invoke-RestMethod -Uri "$repoApi/pulls/$prNumber/comments?per_page=100&page=$page" -Headers $commentHeaders | |
| if ($null -eq $pageComments -or $pageComments.Count -eq 0) | |
| { | |
| break | |
| } | |
| $allReviewComments += $pageComments | |
| if ($pageComments.Count -lt 100) | |
| { | |
| break | |
| } | |
| $page++ | |
| } | |
| foreach ($comment in $allReviewComments) | |
| { |
| $fileInfo = Invoke-RestMethod ` | ||
| -Uri "$repoApi/contents/$($filePath)?ref=$branch" ` | ||
| -Headers $headers | ||
| $remoteSha = $fileInfo.sha | ||
|
|
||
| if ($localSha -eq $remoteSha) | ||
| { | ||
| Write-Output -InputObject 'No licence changes to commit.' | ||
| return | ||
| } | ||
|
|
||
| # Get the branch HEAD OID for the commit mutation. | ||
| $refInfo = Invoke-RestMethod -Uri "$repoApi/git/ref/heads/$branch" -Headers $headers |
There was a problem hiding this comment.
Missing error handling: The Invoke-RestMethod calls on lines 26-28 and 38 don't have error handling. If the GitHub API returns an error (e.g., branch doesn't exist, authentication failure, rate limiting), the script will terminate with a non-descriptive error. Consider adding try-catch blocks around these API calls with meaningful error messages.
| } | ||
|
|
||
| # Get the source branch name (strip refs/heads/ prefix). | ||
| $branch = $Env:SYSTEM_PULLREQUEST_SOURCEBRANCH -replace '^refs/heads/', '' |
There was a problem hiding this comment.
Missing validation: The script uses $Env:SYSTEM_PULLREQUEST_SOURCEBRANCH without checking if it's null or empty. If this environment variable is not set (e.g., when running outside Azure DevOps context), the regex replacement will operate on an empty string, potentially causing unexpected behavior. Consider adding validation to ensure the variable is set before using it.
| $branch = $Env:SYSTEM_PULLREQUEST_SOURCEBRANCH -replace '^refs/heads/', '' | |
| $sourceBranch = $Env:SYSTEM_PULLREQUEST_SOURCEBRANCH | |
| if ([string]::IsNullOrWhiteSpace($sourceBranch)) | |
| { | |
| Write-Error -Message 'SYSTEM_PULLREQUEST_SOURCEBRANCH environment variable is not set. This script must be run in an Azure DevOps pull request context.' | |
| exit 1 | |
| } | |
| $branch = $sourceBranch -replace '^refs/heads/', '' |
| Write-Output -InputObject 'Re-truncated LICENSE.txt for forced regeneration.' | ||
| } | ||
|
|
||
| Write-Output -InputObject 'Licence generation required.' |
There was a problem hiding this comment.
Inconsistent spelling: "Licence" should be "License" to match the American English spelling used throughout the rest of the codebase.
| exit 1 | ||
| } | ||
|
|
||
| Write-Output -InputObject "Licence notices committed via GitHub API (signed): $($response.data.createCommitOnBranch.commit.url)" |
There was a problem hiding this comment.
Inconsistent spelling: "Licence" should be "License" to match the American English spelling used throughout the rest of the codebase.
| $timeline = Invoke-RestMethod -Uri $timelineUrl -Headers $headers | ||
| $noticeRecord = $timeline.records | | ||
| Where-Object { $_.name -eq 'Generate NOTICE File' } | ||
|
|
||
| $errorCount = $noticeRecord.errorCount | ||
| $warningCount = $noticeRecord.warningCount | ||
|
|
||
| # Determine if we should post a PR comment. | ||
| $prNumber = $Env:SYSTEM_PULLREQUEST_PULLREQUESTNUMBER | ||
| if (-not [string]::IsNullOrWhiteSpace($prNumber)) | ||
| { | ||
| $repoApi = 'https://api.github.com/repos/microsoft/PR-Metrics' | ||
| $commentHeaders = @{ | ||
| Authorization = "token $Env:GITHUB_TOKEN" | ||
| Accept = 'application/vnd.github.v3+json' | ||
| } | ||
|
|
||
| # Get the PR's HEAD SHA for file-level review comments. | ||
| $prInfo = Invoke-RestMethod -Uri "$repoApi/pulls/$prNumber" -Headers $commentHeaders | ||
| $headSha = $prInfo.head.sha | ||
|
|
||
| # Remove previous licence generation review comments on LICENSE.txt. | ||
| $errorTemplate = (Get-Content -Path '.github/workflows/support/license-generation-error.md' -Raw).Trim() | ||
| $warningTemplate = (Get-Content -Path '.github/workflows/support/license-generation-warning.md' -Raw).Trim() | ||
| $reviewComments = Invoke-RestMethod -Uri "$repoApi/pulls/$prNumber/comments?per_page=100" -Headers $commentHeaders | ||
| foreach ($comment in $reviewComments) | ||
| { | ||
| if ($comment.path -eq 'src/LICENSE.txt' -and | ||
| ($comment.body.StartsWith($errorTemplate) -or $comment.body.StartsWith($warningTemplate))) | ||
| { | ||
| Invoke-RestMethod -Method Delete -Uri $comment.url -Headers $commentHeaders |
There was a problem hiding this comment.
Missing error handling: The Invoke-RestMethod calls (lines 11, 29, 35, 41) don't have error handling. If the Azure DevOps or GitHub API returns an error (e.g., authentication failure, rate limiting, network issues), the script will terminate with a non-descriptive error. Consider adding try-catch blocks around these API calls with meaningful error messages.
| # ADO mode: guard check + optional re-truncation. | ||
| if ($hasNotices -and -not $Force) | ||
| { | ||
| Write-Output -InputObject 'Licence notices present. Skipping generation.' |
There was a problem hiding this comment.
Inconsistent spelling: "Licence" should be "License" to match the American English spelling used throughout the rest of the codebase.
|
|
||
| if ($localSha -eq $remoteSha) | ||
| { | ||
| Write-Output -InputObject 'No licence changes to commit.' |
There was a problem hiding this comment.
Inconsistent spelling: "licence" should be "license" to match the American English spelling used throughout the rest of the codebase.
- Introduced Push-SignedCommit.ps1 to handle signed commits for license updates. - Added Test-LicensesResult.ps1 for checking license generation results. - Updated template.yml to integrate new scripts and improve license handling workflow.
- Renamed `$branchRef` to `$branchReference` for clarity. - Corrected spelling of "licence" to "license" in output messages.
- Enhanced Get-BranchName function with detailed synopsis. - Updated Get-GitBlobSha function to include parameter documentation. - Refactored New-SignedCommit function for clarity and consistency.
| ($comment.body.StartsWith($errorTemplate) -or $comment.body.StartsWith($warningTemplate))) | ||
| { | ||
| Invoke-RestMethod -Method Delete -Uri $comment.url -Headers $commentHeaders | ||
| Write-Output -InputObject "Deleted previous licence review comment: $($comment.id)" |
There was a problem hiding this comment.
Use American spelling "license" instead of British spelling "licence" to maintain consistency with the codebase.
| } | ConvertTo-Json | ||
| Invoke-RestMethod -Method Post -Uri "$repoApi/pulls/$prNumber/comments" -Headers $commentHeaders -Body $body -ContentType 'application/json' | ||
| $posted = $true | ||
| Write-Output -InputObject 'Posted licence comment on LICENSE.txt.' |
There was a problem hiding this comment.
Use American spelling "license" instead of British spelling "licence" to maintain consistency with the codebase.
| { | ||
| $body = @{ body = $commentBody } | ConvertTo-Json | ||
| Invoke-RestMethod -Method Post -Uri "$repoApi/issues/$prNumber/comments" -Headers $commentHeaders -Body $body -ContentType 'application/json' | ||
| Write-Output -InputObject 'Posted licence comment on PR.' |
There was a problem hiding this comment.
Use American spelling "license" instead of British spelling "licence" to maintain consistency with the codebase.
| $comment.body.StartsWith($warningTemplate)) | ||
| { | ||
| Invoke-RestMethod -Method Delete -Uri $comment.url -Headers $commentHeaders | ||
| Write-Output -InputObject "Deleted previous licence issue comment: $($comment.id)" |
There was a problem hiding this comment.
Use American spelling "license" instead of British spelling "licence" to maintain consistency with the codebase.
- Removed unnecessary synopsis comments from functions. - Updated license spelling for consistency. - Enhanced clarity in comment messages.
- Consolidate logic for fetching pull request number. - Remove outdated comments more efficiently. - Simplify license comment submission process. - Enhance error and warning handling with clearer structure.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
.github/workflows/release-publish.yml:76
Get-Contentreturns an array of lines; becauserelease-publish-trigger.txtcurrently contains a blank second line,$versionbecomes an array andVERSION=v$versioncan end up with an unexpected trailing space/newline, which can break tag creation. Read the version as a single trimmed string (e.g.,Get-Content -Raw+.Trim()orGet-Content | Select-Object -First 1).
| } | ||
|
|
||
| # Set output for downstream steps. | ||
| $licensesResult = $noticeRecord.errorCount -le 0 |
There was a problem hiding this comment.
LICENSES_OK is set based only on errorCount, but earlier logic treats both errors and warnings as requiring a PR comment. With the current output, a warning-only run will still set LICENSES_OK=true, allowing downstream steps to append/commit potentially incomplete license output. Consider including warningCount in the success criteria (or otherwise aligning LICENSES_OK with what you consider a blocking condition).
| $licensesResult = $noticeRecord.errorCount -le 0 | |
| $licensesResult = ($noticeRecord.errorCount -le 0 -and $noticeRecord.warningCount -le 0) |
| cosign verify-blob ms-omex.PRMetrics.vsix \ | ||
| --bundle ms-omex.PRMetrics.vsix.sigstore.json \ | ||
| --certificate-identity-regexp="^https://github.com/microsoft/PR-Metrics/.github/workflows/release-phase-3\.yml@refs/heads/main$" \ | ||
| --certificate-identity-regexp="^https://github.com/microsoft/PR-Metrics/.github/workflows/release-publish\.yml@refs/heads/main$" \ | ||
| --certificate-oidc-issuer="https://token.actions.githubusercontent.com" |
There was a problem hiding this comment.
The --certificate-identity-regexp here contains .github with an unescaped .. In a regex, . matches any character, so this is less strict than intended and could match unintended identities. Escape the dot (i.e., \.github) to keep the identity check precise.
- Replace error and warning template checks with a single marker for outdated comments. - Streamline the removal of outdated comments in both issue and review contexts. - Suppress output for deleted comments to reduce clutter in logs. - Update license generation instructions for clarity and conciseness.
- Change parameter type from PSObject to PSCustomObject for clarity. - Refine licenses result logic to check both error and warning counts.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/release-publish.yml:13
- Because this workflow triggers on changes to
release-publish-trigger.txt, introducing/renaming the trigger file in a migration PR will cause a publish run on the merge commit. To avoid accidental/duplicate releases, consider adding an early check that the requested tag doesn’t already exist (and exit successfully if it does), or otherwise gating the job so only the release automation updates can trigger it.
- Remove warning count check from licenses result evaluation - Streamline output variable assignment for clarity
- Introduced a new workflow for release initiation. - Configured versioning and environment variables. - Added steps for version validation, update, and dependency management.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
.github/workflows/release-publish.yml:76
- The version read from the trigger file may contain trailing whitespace or newlines. Consider using
.Trim()to remove any leading or trailing whitespace to ensure the version string is clean before using it in the GitHub release creation. This would prevent issues like creating a release with a tag like "v1.7.11\n" instead of "v1.7.11".
| $separatorIndex = ($lines | Select-String -Pattern '^-+$' | Select-Object -First 1).LineNumber | ||
| if ($null -eq $separatorIndex) | ||
| { | ||
| throw 'No separator line.' | ||
| } | ||
|
|
||
| $remainingLines = $lines[$separatorIndex..($lines.Count - 1)] | ||
| $nonBlankLines = $remainingLines | Where-Object { -not [string]::IsNullOrWhiteSpace($_) } | ||
| return @($nonBlankLines).Count -gt 0 | ||
| } | ||
|
|
||
| function Remove-Licenses | ||
| { | ||
| [CmdletBinding(SupportsShouldProcess)] | ||
| param( | ||
| [Parameter(Mandatory)] | ||
| [string]$Path | ||
| ) | ||
|
|
||
| $lines = Get-Content -Path $Path | ||
| $separatorIndex = ($lines | Select-String -Pattern '^-+$' | Select-Object -First 1).LineNumber |
There was a problem hiding this comment.
According to the codebase convention (src/LICENSE.txt:25-46), LICENSE.txt contains two separator lines, and dependency licenses begin after the second separator (after 'NOTICES AND INFORMATION'). This script only finds the first separator line using Select-Object -First 1, which may incorrectly truncate at the wrong location. The script should find the separator line immediately before 'NOTICES AND INFORMATION', not just the first separator line in the file.
…ldProcess - Renamed Remove-OutdatedComments to Remove-OutdatedComment for consistency. - Updated Remove-Licenses to Remove-License for clarity. - Added CmdletBinding with SupportsShouldProcess to New-SignedCommit and Remove-OutdatedComment functions.
| env: | ||
| major: 1 | ||
| minor: 7 | ||
| patch: 11 |
There was a problem hiding this comment.
The patch version should be 12, not 11. The removed release-phase-1.yml file had patch: 12, and this consolidation should preserve that version number. Setting it to 11 means the next release will be 1.7.12 instead of 1.7.13, which appears to be a step backward.
| patch: 11 | |
| patch: 12 |
| ) | ||
|
|
||
| $lines = Get-Content -Path $Path | ||
| $separatorIndex = ($lines | Select-String -Pattern '^-+$' | Select-Object -First 1).LineNumber |
There was a problem hiding this comment.
This logic incorrectly finds the FIRST separator line (line 25 in LICENSE.txt) instead of the SECOND separator line (line 44). According to the repository's license structure, dependency licenses begin after the second separator line that comes after the "NOTICES AND INFORMATION" section. Using the first separator will incorrectly truncate the license file, removing critical Microsoft license notice text. The script should find the second separator line or search for the separator that comes after "NOTICES AND INFORMATION".
| ) | ||
|
|
||
| $lines = Get-Content -Path $Path | ||
| $separatorIndex = ($lines | Select-String -Pattern '^-+$' | Select-Object -First 1).LineNumber |
There was a problem hiding this comment.
Same issue as line 19 - this incorrectly finds the FIRST separator line instead of the SECOND separator line. This will cause incorrect truncation of the license file, removing essential Microsoft license notice text.
|
|
||
| if ($PSCmdlet.ShouldProcess($Path, 'Remove licenses')) | ||
| { | ||
| $truncated = $lines[0..($separatorIndex - 1)] + '' |
There was a problem hiding this comment.
The LineNumber property from Select-String is 1-based, but PowerShell array indexing is 0-based. When accessing $lines[0..($separatorIndex - 1)], this includes the separator line itself. For example, if the separator is at line 25 (LineNumber=25), the array index is 24, and $lines[0..24] includes 25 elements (indices 0-24), which includes the separator. This should be $lines[0..($separatorIndex - 2)] to exclude the separator line. Combined with the bug that finds the first separator instead of the second, this will cause significant truncation issues.
2 participants
Summary
Testing
Test Types
Unit Test Coverage
Other Test Details
Screenshots