-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Ensure we fetch all PR detail data #122078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug introduced in a previous cleanup where the JSON fields from GitHub CLI calls weren't properly consolidated. The PR ensures all necessary PR detail data is fetched from a single gh pr view call and updates all references to use the correct data source.
Key Changes
- Modified
gh pr listto fetch only thenumberfield (line 777) to reduce unnecessary data transfer - Updated
gh pr viewto fetch all required fields in one call (line 794) - Changed all references from
$prto$prDetailDatathroughout the script (lines 840-901)
Comments suppressed due to low confidence (1)
eng/breakingChanges/breaking-change-doc.ps1:790
- The variable
$pr.titleis not available at this point. When using the query mode (line 777),$pronly contains thenumberfield. In single PR mode (line 771),$pris a hashtable with onlynumber. Thetitlefield is only available after line 795 in$prDetailData. This should be changed to either omit the title in the log message or use$prDetailData.titleafter it's fetched.
Write-Host " Collecting data for PR #$($pr.number): $($pr.title)" -ForegroundColor Gray
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
|
Thank you for the review. I checked https://github.com/dotnet/runtime/actions/workflows/breaking-change-doc.yml?query=is%3Afailure to see if any other PRs need a manual rerun and don't see any. |
As part of the cleanup in PR d27e210#diff-2ba828c15ad12c13f308f51998a4c9240d7c7325135ff7eaaff120e20dd61166L772
I tried to reduce the number of CLI calls when a PR number was specified. When doing so I didn't consolidate the JSON fields, nor did I update the source of the info later in the script.
This addresses both problems. I've validated with local runs - example - #121938 (comment)