-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Sync eng/common directory with azure-sdk-tools for PR 13005 #47355
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
base: main
Are you sure you want to change the base?
Sync eng/common directory with azure-sdk-tools for PR 13005 #47355
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 syncs the eng/common directory with azure-sdk-tools PR #13005, adding support for tracking TypeSpec specification project paths in package work items. The changes introduce a new SpecProjectPath property that is read from tsp-location.yaml files and propagated through the build and release pipeline to Azure DevOps work items.
Key changes:
- Added
SpecProjectPathproperty toPackagePropsclass with automatic loading fromtsp-location.yaml - Extended work item creation/update functions to include and track
SpecProjectPathin Azure DevOps - Added logging output for the new property in package property saving scripts
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| eng/common/scripts/Package-Properties.ps1 | Adds SpecProjectPath property to PackageProps class and loads it from tsp-location.yaml |
| eng/common/scripts/Save-Package-Properties.ps1 | Adds logging statement to output the SpecProjectPath value during package property saving |
| eng/common/scripts/Validate-All-Packages.ps1 | Passes SpecProjectPath parameter to Update-DevOpsReleaseWorkItem function |
| eng/common/scripts/Helpers/DevOps-WorkItem-Helpers.ps1 | Updates work item helper functions to accept, store, and update SpecProjectPath field in Azure DevOps work items; removes title change detection |
| if ($pkgDisplayName -ne $existingItem.fields["Custom.PackageDisplayName"]) { $changedField = "Custom.PackageDisplayName" } | ||
| if ($pkgType -ne [string]$existingItem.fields["Custom.PackageType"]) { $changedField = "Custom.PackageType" } | ||
| if ($pkgNewLibrary -ne $existingItem.fields["Custom.PackageTypeNewLibrary"]) { $changedField = "Custom.PackageTypeNewLibrary" } | ||
| if ($pkgRepoPath -ne $existingItem.fields["Custom.PackageRepoPath"]) { $changedField = "Custom.PackageRepoPath" } |
Copilot
AI
Nov 22, 2025
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.
The change detection logic doesn't include a check for changes to SpecProjectPath. If an existing work item has a different SpecProjectPath value than what's being passed in, it won't be detected as a changed field, and the work item won't be updated.
Consider adding a check similar to the other fields:
if ($specProjectPath -ne $existingItem.fields["Custom.SpecProjectPath"]) { $changedField = "Custom.SpecProjectPath" }| if ($pkgRepoPath -ne $existingItem.fields["Custom.PackageRepoPath"]) { $changedField = "Custom.PackageRepoPath" } | |
| if ($pkgRepoPath -ne $existingItem.fields["Custom.PackageRepoPath"]) { $changedField = "Custom.PackageRepoPath" } | |
| if ($specProjectPath -ne $existingItem.fields["Custom.SpecProjectPath"]) { $changedField = "Custom.SpecProjectPath" } |
| if ((!$workItem.fields.ContainsKey('Custom.SpecProjectPath') -and $packageInfo.SpecProjectPath) -or | ||
| ($workItem.fields.ContainsKey('Custom.SpecProjectPath') -and ($workItem.fields['Custom.SpecProjectPath'] -ne $packageInfo.SpecProjectPath)) | ||
| ) { | ||
| Write-Host "Updating SpecProjectPath to '$($packageInfo.SpecProjectPath)' for work item [$($workItem.id)]" | ||
| UpdateWorkItem ` | ||
| -id $workItem.id ` | ||
| -fields "`"Custom.SpecProjectPath=$($packageInfo.SpecProjectPath)`"" ` | ||
| -outputCommand $false | ||
| } |
Copilot
AI
Nov 22, 2025
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.
The condition logic for updating SpecProjectPath has a potential issue. When $packageInfo.SpecProjectPath is an empty string (not null), the first condition !$workItem.fields.ContainsKey('Custom.SpecProjectPath') -and $packageInfo.SpecProjectPath will evaluate to false (because empty string is falsy in PowerShell), but the second condition will try to compare against the empty string.
Consider being more explicit about null/empty checks:
if ((![string]::IsNullOrEmpty($packageInfo.SpecProjectPath)) -and
((!$workItem.fields.ContainsKey('Custom.SpecProjectPath')) -or
($workItem.fields['Custom.SpecProjectPath'] -ne $packageInfo.SpecProjectPath)))This ensures we only update when there's actually a non-empty value to set, and properly handles both missing fields and value mismatches.
| } | ||
|
|
||
| if (Test-Path (Join-Path $directoryPath 'tsp-location.yaml')) { | ||
| $this.SpecProjectPath = (LoadFrom-Yaml (Join-Path $directoryPath 'tsp-location.yaml')).directory |
Copilot
AI
Nov 22, 2025
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.
The code assumes that the tsp-location.yaml file will have a directory property, but there's no null check if LoadFrom-Yaml returns null or if the returned object doesn't have a directory property. This could cause a runtime error if the YAML file is malformed or doesn't contain the expected structure.
Consider adding validation:
if (Test-Path (Join-Path $directoryPath 'tsp-location.yaml')) {
$tspLocation = LoadFrom-Yaml (Join-Path $directoryPath 'tsp-location.yaml')
if ($tspLocation -and $tspLocation.directory) {
$this.SpecProjectPath = $tspLocation.directory
}
}| $this.SpecProjectPath = (LoadFrom-Yaml (Join-Path $directoryPath 'tsp-location.yaml')).directory | |
| $tspLocation = LoadFrom-Yaml (Join-Path $directoryPath 'tsp-location.yaml') | |
| if ($tspLocation -and $tspLocation.directory) { | |
| $this.SpecProjectPath = $tspLocation.directory | |
| } |
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#13005 See eng/common workflow