Sync eng/common directory with azure-sdk-tools for PR 10286 #45434
Conversation
azure-sdk
added
EngSys
There was a problem hiding this comment.
Pull Request Overview
Sync the eng/common directory with updates from the Azure SDK tools repository to improve correlation tracking and logging, and adjust the pipeline step naming.
- Added correlation IDs and custom headers to APIView and API change detection HTTP calls
- Enhanced logged output by pretty-printing JSON responses
- Updated pipeline step display name for clarity
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| eng/common/scripts/Helpers/ApiView-Helpers.ps1 | Inject x-correlation-id header and pretty-print API review responses |
| eng/common/scripts/Detect-Api-Changes.ps1 | Inject x-correlation-id, refine headers, and pretty-print JSON |
| eng/common/pipelines/templates/steps/detect-api-changes.yml | Renamed displayName from “Detect API changes” to “Create APIView if API has changes” |
|
|
||
| $correlationId = [System.Guid]::NewGuid().ToString() | ||
| $headers = @{ | ||
| "Content-Type" = "application/json" |
There was a problem hiding this comment.
Using a 'Content-Type' header on a GET request is misleading—consider replacing it with an 'Accept: application/json' header or removing it entirely.
| "Content-Type" = "application/json" | |
| "Accept" = "application/json" |
Copilot uses AI. Check for mistakes.
| $StatusCode = $Response.StatusCode | ||
| $responseContent = $Response.Content | ConvertFrom-Json | ConvertTo-Json -Depth 10 | ||
| LogSuccess $responseContent |
There was a problem hiding this comment.
[nitpick] LogSuccess only outputs the JSON body. Including the HTTP status code in this log would make troubleshooting easier and consistent with other calls.
| LogSuccess $responseContent | |
| LogSuccess "Status Code: $StatusCode, Response: $responseContent" |
Copilot uses AI. Check for mistakes.
| $responseContent = $Response.Content | ConvertFrom-Json | ConvertTo-Json -Depth 10 | ||
| LogSuccess "Status Code: $($response.StatusCode)`nAPI review request created successfully.`n$($responseContent)" | ||
| } | ||
| elseif ($response.StatusCode -eq 208) { | ||
| LogSuccess "Status Code: $($response.StatusCode)`nThere is no API change compared with the previous version." | ||
| $responseContent = $Response.Content | ConvertFrom-Json | ConvertTo-Json -Depth 10 |
There was a problem hiding this comment.
[nitpick] Variable casing is inconsistent—$Response vs $response. Unifying on one spelling improves readability and reduces potential confusion.
Copilot uses AI. Check for mistakes.
chidozieononiwu
deleted the
sync-eng/common-apiViewMVCtoAPIControllers-10286
branch
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#10286 See eng/common workflow