This repository was archived by the owner on Feb 12, 2024. It is now read-only.
feat: update DAG API to match [email protected] changes #3917
fix(ci): fail-fast: false
I imagine we want to see full list of things that pass and don't. Failing fast hides some problems by cancelling all matrix jobs on the first failure.
- Loading branch information
commit c6816a4676a1bfc77c462fdbd0fcb33eb5f8883e
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
AFAIK gh orgs only have a certain number of workers available for actions - this may end up hogging resources and preventing other builds from running when we know a given CI run will fail.
Uh oh!
There was an error while loading. Please reload this page.
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.
If we truly care about not using too many workers then fail-fast does not buy us much.
We should limit the number of concurrent jobs with
concurrency– did a poc in 50cbc1f where we limit number of test workers consumed per github revision, which makes PRs independent of each other.Bit confused why CI stopped passing without any package.json changes – is lerna setup fuzzy like this by design?
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.
sorry, this wasn't meant to be here for the final version - I turned this off because of the examples failures, which are currently commented out and need to be addressed before merge but I'm not sure how to do that dance when they depend on eachother being fixed
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 should be optimistic that builds will complete successfully, so they should consume as many resources as necessary to do that expediently (within reason). If they fail, they should not stand in the way of other builds completing, so should fail fast.
Not sure what reasonable is yet but so far so good.
Right now it's failing because the types in
iso-random-streamare broken - needs hugomrdias/iso-random-stream#91We can check out branches of the example tests with fixes applied which should get the build passing.
e.g. in
.github/workflows/test.ymlwould become:
If that's not practical, I think it's ok to merge with the example tests disabled, then fix the examples and re-enable them in a follow up PR. As long as there isn't a massive lag between the two.