-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Ensure Get-AllPkgProperties
handles ci.yml
below sdk/<service>
#43124
base: main
Are you sure you want to change the base?
Conversation
@@ -29,7 +29,7 @@ function Get-AllPackageInfoFromRepo([string]$serviceDirectory = $null) { | |||
if ($serviceDirectory) { | |||
$searchPath = Join-Path $sdkRoot $serviceDirectory | |||
Write-Host "searchPath=$searchPath" | |||
[array]$ymlFiles = Get-ChildItem -Path $searchPath "ci*.yml" | Where-Object { $_.PSIsContainer -eq $false} | |||
[array]$ymlFiles = Get-ChildItem -Recurse -Path $searchPath "ci*.yml" | Where-Object { $_.PSIsContainer -eq $false} |
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.
We had the reverse issue in the go repo, where there were nested ci.yml files that weren't intended to be sucked up by parent level pipelines. Seems there are a couple cases, like with maps, where there is both a sdk/maps/ci.yml
file AND a sdk/maps/<sub dir>/ci.yml
file. Not sure if this will cause unintended pipeline consequences there vs. what's intended in communication. Just FYI
./sdk/maps/azure-maps-geolocation/ci.yml
./sdk/maps/azure-maps-route/ci.yml
./sdk/maps/azure-maps-traffic/ci.yml
./sdk/maps/azure-maps-render/ci.yml
./sdk/maps/azure-maps-timezone/ci.yml
./sdk/maps/azure-maps-weather/ci.yml
./sdk/maps/azure-maps-search/ci.yml
./sdk/spring/spring-cloud-azure-starter-monitor/ci.yml
./sdk/communication/azure-communication-callingserver/ci.yml
./sdk/communication/azure-communication-sms/ci.yml
./sdk/communication/azure-communication-callautomation/ci.yml
./sdk/communication/azure-resourcemanager-communication/ci.yml
./sdk/communication/azure-communication-chat/ci.yml
./sdk/communication/azure-communication-rooms/ci.yml
./sdk/communication/azure-communication-email/ci.yml
./sdk/communication/azure-communication-messages/ci.yml
./sdk/communication/azure-communication-common/ci.yml
./sdk/communication/azure-communication-jobrouter/ci.yml
./sdk/communication/azure-communication-identity/ci.yml
./sdk/communication/azure-communication-phonenumbers/ci.yml
./sdk/identity/azure-identity-extensions/ci.yml
./sdk/eventgrid/azure-resourcemanager-eventgrid/ci.yml
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.
@benbp you're absolutely right and I'm going to wait to merge this till @JimSuplizio returns to merge this PR. Nothing is super urgent, I'm just trying to help @mssfang
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.
Yeah. Thank you for the help @scbedd It unblocks my release pipeline. No longer see those occurred errors.
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.
@scbedd and @mssfang, this isn't the right fix. When I wrote this function a few weeks back, the lack of the -Recurse
in the command was intentional. If -Recurse is added, it'll break things, and we'll end up with exact problem I'd just fixed which was packageInfo generation was too greedy. In this particular case, if -Recurse
was used it would get all of the communication libraries' packageInfo files, including ones that weren't release.
See title.
Before:
sdk/communication/ci.*.yml
After:
sdk/communication/**/ci.*.yml
Example build failure