Add field for Spec Project Path to Package Work Item #13005
Conversation
|
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: |
danieljurek
changed the title
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
There was a problem hiding this comment.
Pull request overview
This PR adds a SpecProjectPath field to Package Work Items to track the path from the root of the azure-rest-api-specs repository to the spec project. The field is populated by reading from a tsp-location.yaml file in the package directory if it exists.
Key changes:
- Added
SpecProjectPathproperty to thePackagePropsclass, reading fromtsp-location.yamlif present - Updated DevOps work item helpers to include
Custom.SpecProjectPathfield when creating/updating work items - Added test infrastructure to enable easier scenario file updates with a flag
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/common/scripts/Package-Properties.ps1 | Added SpecProjectPath property to PackageProps class and logic to read from tsp-location.yaml |
| eng/common/scripts/Validate-All-Packages.ps1 | Updated function call to pass specProjectPath parameter |
| eng/common/scripts/Save-Package-Properties.ps1 | Added logging output for SpecProjectPath field |
| eng/common/scripts/Helpers/DevOps-WorkItem-Helpers.ps1 | Added Custom.SpecProjectPath field handling in work item functions and removed title comparison check |
| eng/common-tests/matrix-generator/pr-matrix-samples/pr-matrix-generation-acceptance.tests.ps1 | Added $updateScenarios flag and logic to update scenario files programmatically |
| eng/common-tests/matrix-generator/pr-matrix-samples/python_scenarios.json | Updated test expectations to include SpecProjectPath: null or actual spec paths |
| eng/common-tests/matrix-generator/pr-matrix-samples/net_scenarios.json | Updated test expectations to include SpecProjectPath: null |
| eng/common-tests/matrix-generator/pr-matrix-samples/js_scenarios.json | Updated test expectations to include SpecProjectPath: null or actual spec paths |
| eng/common-tests/matrix-generator/pr-matrix-samples/java_scenarios.json | Updated test expectations to include SpecProjectPath: null |
| eng/common-tests/matrix-generator/pr-matrix-samples/go_scenarios.json | Updated test expectations to include SpecProjectPath: null |
| | ForEach-Object { Get-Content -Raw $_ | ConvertFrom-Json -AsHashtable } | ||
| | Sort-Object -Property Name | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] Extra blank line should be removed for consistency with the rest of the code.
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] There are extra blank lines here. Consider removing these unnecessary blank lines for code cleanliness.
| if ($specProjectPath) { | ||
| $fields += "`"Custom.SpecProjectPath=${specProjectPath}`"" | ||
| } |
There was a problem hiding this comment.
The SpecProjectPath field is added to the work item fields but is not included in the change detection logic (lines 546-553). This means that if only the SpecProjectPath changes for an existing work item, the work item will not be updated. Consider adding a check like if ($specProjectPath -ne $existingItem.fields["Custom.SpecProjectPath"]) { $changedField = "Custom.SpecProjectPath" } after line 553.
| } | ||
|
|
||
| if (Test-Path (Join-Path $directoryPath 'tsp-location.yaml')) { | ||
| $this.SpecProjectPath = (LoadFrom-Yaml (Join-Path $directoryPath 'tsp-location.yaml')).directory |
There was a problem hiding this comment.
The code doesn't handle the case where LoadFrom-Yaml returns $null (which happens if the YAML file fails to parse) or where the parsed YAML object doesn't contain a directory field. This could cause a null reference error. Consider adding null-checking:
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 | |
| } |
| if ($updateScenarios) { | ||
| $netScenarios | ConvertTo-Json -Depth 100 | Set-Content (Join-Path $PSScriptRoot net_scenarios.json) | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] Extra blank line should be removed for consistency with the rest of the code.
Partial #12992
When running validate-all-packages.yml, add
SpecProjectPathif a path can be found from atsp-location.ymlin theDirectoryPathof the package.This PR changes the JSON output of package metadata which causes failures in the matrix generator scripts so it includes updates to the expected outputs of those scripts and an easier way to update the files containing expectations going forward.