-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Sync eng/common directory with azure-sdk-tools for PR 13005 #36721
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 #36721
Conversation
eshanhasitha
left a comment
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.
✅
2347f18 to
fb46836
Compare
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 project locations. The changes enable the SDK build and release infrastructure to associate packages with their corresponding spec projects in the azure-rest-api-specs repository.
Key Changes:
- Added
SpecProjectPathproperty toPackagePropsclass to store the path from the root of azure-rest-api-specs repo to the spec project - Updated DevOps work item helpers to read, store, and update the
Custom.SpecProjectPathfield in Azure DevOps work items - Modified package validation scripts to pass the spec project path through the release workflow
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eng/common/scripts/Package-Properties.ps1 | Added SpecProjectPath property to PackageProps class and logic to read it from tsp-location.yaml file |
| eng/common/scripts/Save-Package-Properties.ps1 | Added logging output for the Spec Project Path property |
| eng/common/scripts/Validate-All-Packages.ps1 | Updated CreateUpdatePackageWorkItem to pass specProjectPath parameter to Update-DevOpsReleaseWorkItem |
| eng/common/scripts/Helpers/DevOps-WorkItem-Helpers.ps1 | Added Custom.SpecProjectPath field handling in work item queries, creation, and updates including new parameter in Update-DevOpsReleaseWorkItem function |
| } | ||
|
|
||
| if (Test-Path (Join-Path $directoryPath 'tsp-location.yaml')) { | ||
| $this.SpecProjectPath = (LoadFrom-Yaml (Join-Path $directoryPath 'tsp-location.yaml')).directory |
Copilot
AI
Nov 25, 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.
Potential null reference exception: LoadFrom-Yaml can return null if the file doesn't exist or fails to parse (as seen in the function definition). Accessing .directory on a null object will cause a runtime error.
Consider adding null-checking:
$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 | |
| } |
|
|
||
|
|
||
|
|
||
| if ((!$workItem.fields.ContainsKey('Custom.SpecProjectPath') -and $packageInfo.SpecProjectPath) -or |
Copilot
AI
Nov 25, 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.
Logic issue with empty string handling: The condition checks if $packageInfo.SpecProjectPath is truthy, but the parameter default value is an empty string "" (line 1265). In PowerShell, an empty string is falsy, but when the SpecProjectPath is explicitly set to an empty string in $packageInfo (line 1289), the condition on line 1338 may not behave as intended if you want to distinguish between "not set" (null) and "set to empty".
If empty string is a valid value that should trigger updates, consider using -and ![string]::IsNullOrEmpty($packageInfo.SpecProjectPath) or similar. If null should be used to indicate "not set", consider changing the parameter default to $null instead of "" on line 1265.
| if ((!$workItem.fields.ContainsKey('Custom.SpecProjectPath') -and $packageInfo.SpecProjectPath) -or | |
| if ((!$workItem.fields.ContainsKey('Custom.SpecProjectPath') -and -not [string]::IsNullOrEmpty($packageInfo.SpecProjectPath)) -or |
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#13005 See eng/common workflow