Added packageInfo to Find-java-Artifacts-For-Apireview function #46958
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Find-java-Artifacts-For-Apireview function to support artifact discovery using package metadata. The enhancement allows the function to locate artifacts based on groupId and artifactName from a packageInfo parameter, providing more precise artifact location capabilities.
Key changes:
- Added optional
$packageInfoparameter to enable metadata-driven artifact discovery - Implemented conditional logic to use package metadata when available, falling back to existing discovery methods
- Added function pointer documentation comments for better code organization
eng/scripts/Language-Settings.ps1
Outdated
| $artifactPath = Join-Path $artifactDir "io.clientcore*" $pkgName | ||
| $files += @(Get-ChildItem -Recurse "${artifactPath}" | Where-Object -FilterScript {$_.Name.EndsWith("sources.jar")}) | ||
| if ($packageInfo) { | ||
| $artifactPath = Join-Path $artifactDir $packageInfo.Group $packageInfo.ArtifactName |
There was a problem hiding this comment.
The property access uses $packageInfo.Group but based on the context and typical Java artifact structure, this should likely be $packageInfo.GroupId to match Maven/Gradle conventions for group identifiers.
| $artifactPath = Join-Path $artifactDir $packageInfo.Group $packageInfo.ArtifactName | |
| $artifactPath = Join-Path $artifactDir $packageInfo.GroupId $packageInfo.ArtifactName |
Copilot uses AI. Check for mistakes.
| $files += @(Get-ChildItem -Recurse "${artifactPath}" | Where-Object -FilterScript {$_.Name.EndsWith("sources.jar")}) | ||
| if ($packageInfo) { | ||
| $artifactPath = Join-Path $artifactDir $packageInfo.Group $packageInfo.ArtifactName | ||
| $files = @(Get-ChildItem "${artifactPath}" | Where-Object -FilterScript {$_.Name.EndsWith("sources.jar")}) |
There was a problem hiding this comment.
The Get-ChildItem command may fail if the $artifactPath directory doesn't exist, but there's no error handling. Consider adding -ErrorAction SilentlyContinue or checking if the path exists before attempting to access it.
Copilot uses AI. Check for mistakes.
eng/scripts/Language-Settings.ps1
Outdated
| # function is used to filter packages to submit to API view tool | ||
| function Find-java-Artifacts-For-Apireview($artifactDir, $pkgName) | ||
| # Function pointer name: FindArtifactForApiReviewFn | ||
| function Find-java-Artifacts-For-Apireview($artifactDir, $pkgName, $packageInfo = $null) |
There was a problem hiding this comment.
| function Find-java-Artifacts-For-Apireview($artifactDir, $pkgName, $packageInfo = $null) | |
| function Find-java-Artifacts-For-Apireview($artifactDir, $packageInfo) |
I thought that is what you would have based on your common changes.
There was a problem hiding this comment.
To avoid breaking change and down time, my roll out plan includes three steps:
- merge this PR by just adding
packageInfoparameter. - merge the PR of
apiviewpipeline&script changes which takepackageInfoparameter - remove the redundant
pkgNamefrom this functionFind-java-Artifacts-For-Apireview
What do you think?
There was a problem hiding this comment.
I'm fine with that but I though your change in eng/common already accounted for this by looking at the parameters and calling the correct function.
There was a problem hiding this comment.
#2 is another PR I haven't created yet.
There was a problem hiding this comment.
Created Azure/azure-sdk-tools#12467 for step#2. I can update this PR after merge the former PR for the changes in API-review templates.
2 participants
Enable the artifact discovery based on the
groupIdandartifactNamefrom thepackageInfo.Parent issue is at Azure/azure-sdk-tools#10219