Sync eng/common directory with azure-sdk-tools for PR 12467 #25422
Conversation
azure-sdk
added
EngSys
There was a problem hiding this comment.
Pull Request Overview
This PR synchronizes the eng/common directory with azure-sdk-tools PR 12467, introducing improvements to the API review creation process. The changes refactor parameter handling to support package information files directly while maintaining backward compatibility with existing workflows.
Key Changes
- Modified function signatures to accept
packageInfoobjects instead of separate package name parameters - Added parameter checking logic to support both legacy and new parameter patterns
- Introduced
PackageInfoFilesparameter as a new way to specify packages for API review
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| eng/common/scripts/Detect-Api-Changes.ps1 | Removed $pkgArtifactName parameter from function call to align with refactored signature |
| eng/common/scripts/Create-APIReview.ps1 | Refactored to accept packageInfo objects throughout, added parameter priority logic, and introduced PackageInfoFiles parameter support |
| eng/common/pipelines/templates/steps/create-apireview.yml | Added typed parameter definitions and PackageInfoFiles parameter to pipeline template |
| if ($supportsPackageInfoParam) { | ||
| LogInfo "Calling $FindArtifactForApiReviewFn with packageInfo parameter" | ||
| $packages = &$FindArtifactForApiReviewFn $ArtifactPath $pkgArtifactName $packageInfo | ||
| $packages = &$FindArtifactForApiReviewFn $ArtifactPath $packageInfo |
There was a problem hiding this comment.
The function call removes the $pkgArtifactName parameter but doesn't check whether the function expects it. In Create-APIReview.ps1, the same function is called with parameter detection logic (lines 183-201) to determine whether to pass 2 or 3 arguments. This file should implement the same check to avoid breaking calls to functions that still expect $pkgArtifactName.
Copilot uses AI. Check for mistakes.
| # Check if the function supports the packageInfo parameter | ||
| $functionInfo = Get-Command $FindArtifactForApiReviewFn -ErrorAction SilentlyContinue | ||
| $supportsPackageInfoParam = $false | ||
|
|
There was a problem hiding this comment.
Excessive whitespace on line 186. Remove the trailing spaces to maintain consistent code formatting.
Copilot uses AI. Check for mistakes.
| # Call function with appropriate parameters | ||
| if ($supportsPackageInfoParam) { | ||
| LogInfo "Calling $FindArtifactForApiReviewFn with packageInfo parameter" | ||
| $packages = &$FindArtifactForApiReviewFn $ArtifactPath $pkgArtifactName $packageInfo |
There was a problem hiding this comment.
When $supportsPackageInfoParam is true, the function is called with three parameters: $ArtifactPath, $pkgArtifactName, and $packageInfo. However, the parameter name check only verifies that 'packageInfo' exists in the parameter list. This doesn't guarantee the function expects exactly these three parameters in this order. Consider also checking the total parameter count to ensure compatibility.
Copilot uses AI. Check for mistakes.
azure-sdk
force-pushed
the
sync-eng/common-users/raych1/update-api-view-template-12467
branch
from
04b9963 to
85ea89d
Compare
2 participants
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#12467 See eng/common workflow