Add Merge-DevPullRequest cmdlet and update documentation #28718
Conversation
- Introduced `Merge-DevPullRequest` cmdlet to facilitate merging pull requests in the azure-powershell repository. - Updated README.md to include usage instructions for the new cmdlet. - Added entry to CHANGELOG.md for the new feature. - Modified AzDev.psd1 to export the new cmdlet and its alias.
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
There was a problem hiding this comment.
Pull Request Overview
Adds a new Merge-DevPullRequest cmdlet (alias Merge-DevPR) to assist with approving and merging pull requests in the azure-powershell repository. Updates documentation (README, CHANGELOG) and exports the new function and alias in AzDev.psd1.
- Introduces GitHub.psm1 with Merge-DevPullRequest implementation.
- Updates module manifest to load and export the new function and alias.
- Adds README usage and CHANGELOG entry for the new feature.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| tools/AzDev/README.md | Documents new GitHub helper section and usage examples for Merge-DevPullRequest. |
| tools/AzDev/CHANGELOG.md | Adds dated entry describing new cmdlet feature. |
| tools/AzDev/AzDev/GitHub.psm1 | Implements Merge-DevPullRequest cmdlet logic and exports it. |
| tools/AzDev/AzDev/AzDev.psd1 | Registers new nested module, function export, and alias. |
| if ($failedPRs.Count -gt 0) { | ||
| $errorMessage = "Failed to merge $($failedPRs.Count) pull request(s): $($failedPRs.number -join ', ')" | ||
| Write-Error $errorMessage | ||
| throw $errorMessage | ||
| } | ||
|
|
||
| # Return merged PRs in the format shown in README | ||
| return $mergedPRs | ForEach-Object { | ||
| [PSCustomObject]@{ |
There was a problem hiding this comment.
The function throws (line 183) before reaching the return block (line 186) when any PR fails, contradicting the documented behavior that successfully merged PRs are still returned. Move the return construction before the throw or replace the throw with a non-terminating Write-Error so merged PRs are output. Example fix: build the result object, Write-Error if failures exist, then return the result without throwing.
There was a problem hiding this comment.
I think this is valid as there is no wrapping catch.
There was a problem hiding this comment.
Updated: merged PRs are returned before the errors are thrown
| 'No.' = $_.number | ||
| 'Title' = $_.title | ||
| 'CreatedBy' = $_.author.login | ||
| 'CreatedAt' = [DateTime]::Parse($_.createdAt).ToString('M/d/yyyy h:mm:ss tt') |
There was a problem hiding this comment.
[nitpick] Using a culture-dependent format ('M/d/yyyy h:mm:ss tt') produces ambiguous dates and hampers machine parsing. Prefer an invariant/ISO 8601 format such as ToString('yyyy-MM-ddTHH:mm:ssZ') (after ensuring UTC) or return the DateTime object directly for consumers to format.
| 'CreatedAt' = [DateTime]::Parse($_.createdAt).ToString('M/d/yyyy h:mm:ss tt') | |
| 'CreatedAt' = ([DateTime]::Parse($_.createdAt).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ssZ')) |
There was a problem hiding this comment.
@isra-fel I believe this could be a valid point.
| 'No.' = $_.number | ||
| 'Title' = $_.title | ||
| 'CreatedBy' = $_.author.login | ||
| 'CreatedAt' = [DateTime]::Parse($_.createdAt).ToString('M/d/yyyy h:mm:ss tt') |
There was a problem hiding this comment.
[nitpick] Using a culture-dependent format ('M/d/yyyy h:mm:ss tt') produces ambiguous dates and hampers machine parsing. Prefer an invariant/ISO 8601 format such as ToString('yyyy-MM-ddTHH:mm:ssZ') (after ensuring UTC) or return the DateTime object directly for consumers to format.
| 'CreatedAt' = [DateTime]::Parse($_.createdAt).ToString('M/d/yyyy h:mm:ss tt') | |
| 'CreatedAt' = ([DateTime]::Parse($_.createdAt).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ssZ')) |
There was a problem hiding this comment.
| 'CreatedAt' = [DateTime]::Parse($_.createdAt).ToString('M/d/yyyy h:mm:ss tt') | |
| 'CreatedAt' = [DateTime]::Parse($_.createdAt).ToString('M/d/yyyy h:mm:ss tt') |
|
|
||
| With the `-AllArchivePR` switch, the cmdlet lists all the matching PRs, ordered by CreatedAt ascending, and prompts you to confirm before merging. Use the `-Force` switch to skip the confirmation. For safety, this parameter set only supports merging PRs with the "[skip ci]" prefix in the title and created by the `azure-powershell-bot` user, which are the archive PRs for generated modules. | ||
|
|
||
| The cmdlet returns the list of merged PRs. In case any PR fails to merge, an error is thrown, and the successfully merged PRs are still returned. |
There was a problem hiding this comment.
Documentation states merged PRs are still returned when an error occurs, but current implementation throws before returning (see lines 180-188 in GitHub.psm1). Update the code to return results on partial failure or adjust this sentence to reflect the actual behavior.
| The cmdlet returns the list of merged PRs. In case any PR fails to merge, an error is thrown, and the successfully merged PRs are still returned. | |
| The cmdlet returns the list of merged PRs. If any PR fails to merge, an error is thrown and no results are returned. |
Co-authored-by: Copilot <[email protected]>
2 participants
Merge-DevPullRequestcmdlet to facilitate merging pull requests in the azure-powershell repository.Description
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.