[TypeSpec APIView] Ensure apiview only triggers on packages directly changed by the pr #31871
Conversation
ckairen
added
the
Central-EngSys
Next Steps to Merge⌛ Please wait. Next steps to merge this PR are being evaluated by automation. ⌛ |
Generated ApiView
|
|
API change check APIView has identified API level changes in this PR and created following API reviews. |
| @@ -17,12 +17,6 @@ model Employee is TrackedResource<EmployeeProperties> { | |||
|
|
|||
| /** Employee properties */ | |||
| model EmployeeProperties { | |||
There was a problem hiding this comment.
| model EmployeeProperties { | |
| model EmployeeProperties { | |
| /** Age of employee */ | |
| age?: int32; | |
| /** City of employee */ | |
| city?: string; | |
There was a problem hiding this comment.
temp removal for testing
ckairen
requested review from
weshaggard,
mikeharder,
benbp,
raych1 and
maririos
as code owners
|
Do we really want to show entire npm package tree in log? This makes it difficult to read the logs that's more important. |
You can use the log grouping similar to azure-rest-api-specs/eng/scripts/Create-APIView.ps1 Lines 143 to 147 in d7d8b18 which will hide the lengthy logs in a toggle. That way people can still expand to view it if they want to. |
| while ($filePathParts.Length -and !$configFilesInTypeSpecProjects) { | ||
| $filePathParts = $filePathParts | Select-Object -SkipLast 1 | ||
| $typeSpecProjectBaseDirectory = $filePathParts -join [IO.Path]::DirectorySeparatorChar | ||
| $configFilesInTypeSpecProjects = Get-ChildItem -Path $typeSpecProjectBaseDirectory -File "tspconfig.yaml" |
There was a problem hiding this comment.
This will still be an issue. It is possible to have tspconfig.yaml in sub folders for a shared project. Current logic will stop looking further upwards and move to next block to check if current directory has main.tsp. It won't come back and check again the parent path of current file path if there is no main.tsp.
We can simplify this logic that will work for shared projects also.
for each changed files, strip the prefix before `specification/' from path:
- check if current path has tspconfig.yaml and main.tsp.
- If yes, add this folder to project set
- If no, then go upwards and continue step 2 until we match current path to 'specification/[^\/]+/+`
APIView needs to be generated from paths that contain both main.tsp and tspconfig.yaml (at least to retain same behavior as current one.
There was a problem hiding this comment.
Hmm I don't think I understand the exact situation where this might not work.
The code right now is checking all parent directory of the changed file till we reach root.
line 96 $filePathParts = $filePathParts | Select-Object -SkipLast 1 deletes the leaf directory on each iteration
closes #31870