Use full package name in DevOps work item #13076
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances package identification for Java packages by using the full package name (groupId+artifactName) in DevOps work items and related operations.
Key changes:
- Introduces
Get-FullPackageNamefunction to return full package names with proper format for different contexts - Updates
Get-PkgPropertiesto filter by both package name and GroupId when GroupId is provided - Adds GroupId parameter handling across multiple scripts for consistent Java package identification
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/common/scripts/Verify-ChangeLog.ps1 | Adds GroupId parameter and passes it to Get-PkgProperties for proper package filtering |
| eng/common/scripts/Validate-All-Packages.ps1 | Uses Get-FullPackageName to ensure correct package names in DevOps work items and validation outputs |
| eng/common/scripts/Update-ChangeLog.ps1 | Adds GroupId parameter support for consistent package identification |
| eng/common/scripts/Prepare-Release.ps1 | Adds GroupId prompting for Java packages and uses full package names in API review and DevOps updates, fixes typo in prompt message |
| eng/common/scripts/Package-Properties.ps1 | Enhances Get-PkgProperties to filter by GroupId when provided for accurate package matching |
| eng/common/scripts/Helpers/Package-Helpers.ps1 | Introduces Get-FullPackageName helper function with support for different separator formats |
| eng/common/pipelines/templates/steps/verify-changelog.yml | Adds GroupId parameter to YAML template for pipeline consistency |
Comments suppressed due to low confidence (1)
eng/common/scripts/Validate-All-Packages.ps1:199
- Line 199 assigns
$pkgNameto$fullPackageNamebut this value is immediately overwritten on line 205. This line serves no purpose and should be removed to improve code clarity.
$fullPackageName = $pkgName
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
| return | ||
| } | ||
| $pkgName = $pkg.Package | ||
| $pkgGroupId = if ($pkg.PSObject.Properties.Name -contains "GroupId") { $pkg.GroupId } else { $pkg.Group } |
There was a problem hiding this comment.
package csv uses GroupId as the name while PackageInfo and work item use Group. We might want to make them consistent in the future.
There was a problem hiding this comment.
I would prefer to call this GroupId for the new field if possible to make this consistent. Ideally we would also update the PackageProps to rename that to GroupId as well but that might be a little more work.
There was a problem hiding this comment.
If that's the direction, let's make the new field called GroupId in work item. I will create an issue for tracking the future update on the PackageProps.
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
| $fields = @() | ||
| $fields += "`"Language=${lang}`"" | ||
| $fields += "`"Package=${pkgName}`"" | ||
| $fields += "`"GroupId=${pkgGroupId}`"" |
There was a problem hiding this comment.
Should we only include this clause if groupId isn't null?
There was a problem hiding this comment.
I'd rather keeping it updated to the input value even if it's null.
| else { | ||
| $pkgProps = $allPkgProps.Where({ | ||
| ($_.Name -eq $PackageName -or $_.ArtifactName -eq $PackageName) -and | ||
| ($_.PSObject.Properties.Name -contains "Group" -and $_.Group -eq $GroupId) |
There was a problem hiding this comment.
| ($_.PSObject.Properties.Name -contains "Group" -and $_.Group -eq $GroupId) | |
| ($_.PSObject.Properties.Name -contains "GroupId" -and $_.GroupId -eq $GroupId) |
There was a problem hiding this comment.
Nevermind this looks to be a packageprops/info so I guess it would still be Group.
There was a problem hiding this comment.
The input object comes from the PackageProps type and we have to access it by Group.
weshaggard
left a comment
weshaggard
left a comment
There was a problem hiding this comment.
Couple potential clean-ups but otherwise looks good.
|
The following pipelines have been queued for testing: |
Fixed #12507
Key Changes:
groupIdwhen dealing with the dev ops work itemGet-PkgPropertiesto match both of thenameandgroupIdifGroupexists