-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
clean up upstream-dev CI #4599
clean up upstream-dev CI #4599
Conversation
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.
@keewis, thank you for catching and addressing this...
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.
Looks good to me 👍
great, thanks again, @andersy005. Let's merge now and leave the choice between the old and new PR CI to a different PR. |
Of course! I'm curious, are there any benefits to keeping the old azure upstream-dev CI around? |
If I remember correctly, we decided to mark the upstream-dev CI as allowed failure. If there's a way to mark the PR CI as such, I think the only benefit is that a allowed failure of the old azure CI is a bit more visible (see #4584 (comment)). |
If I understand this correctly, this is something that can be done via the branch protection settings from the repo settings. |
That's somewhat different, I think. Branch protection defines CI that has to pass in order to be able to merge the PR, while allowed failures change the result of the CI. For the current implementation of this, see the changes in #4584. Edit: the |
Oooh I see... Thank you for the clarification |
follow-up to #4583, which also added a PR upstream-dev CI. However, because the output of
pytest
has been redirected to a file, the failures are hard to diagnose. The fix is to duplicatestdout
usingtee
.Additionally, we now have two upstream-dev CI, so I guess we should remove the old one?#4584 will probably set the upstream-dev CI toallow_failure: true
, so it might be better to remove the new CI (or to somehow mark the new PR CI withallow_failure: true
while not changing the scheduled CI).