-
Notifications
You must be signed in to change notification settings - Fork 780
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
ci: fix gator cli build #2657
ci: fix gator cli build #2657
Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
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.
LGTM
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.
just two non blocking questions
|
||
# wait for all pids | ||
for pid in ${pids[*]}; do | ||
wait $pid |
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.
two thoughts
- wondering if we want a specific timeout here, besides the overall timeout defined on line 19 in this file for all jobs;
- did we want to fail the job if one of the pid's returns non zero?
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.
- no specific timeout, just timeout for the pipeline itself
- yea that is the proposed behavior in the PR (
set -e
). we are not doing that today so we didn't know release of gator failed
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.
LGTM, thanks for spotting this!
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #2657 +/- ##
==========================================
- Coverage 53.36% 53.23% -0.13%
==========================================
Files 120 120
Lines 10594 10594
==========================================
- Hits 5653 5640 -13
- Misses 4508 4518 +10
- Partials 433 436 +3
Flags with carried forward coverage won't be shown. Click here to find out more. see 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Xander Grzywinski <xandergr@microsoft.com>
What this PR does / why we need it:
This fixes gator cli builds during release and it should mark the job as failed if build fails
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #2656
Special notes for your reviewer:
Will need to be cherry picked to release-3.12 branch after merged