Fix lint CI and split it into multiple jobs #2785
Merged
Conversation
This file contains 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
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Should we just not use |
|
We could! That'll be a lot of jobs though 😅 And the only time consuming one here is the |
ndr-ds
force-pushed
the
11-01-lint_has_been_silently_timing_out
branch
from
3ffafd9
to
1f6268a
Compare
|
Looking at the documentation more, I think that if the last job without |
ndr-ds
force-pushed
the
11-01-lint_has_been_silently_timing_out
branch
6 times, most recently
from
c68473d
to
3e17207
Compare
ndr-ds
commented
Nov 4, 2024
ndr-ds
force-pushed
the
11-01-lint_has_been_silently_timing_out
branch
3 times, most recently
from
2010c9e
to
f9fa551
Compare
ndr-ds
force-pushed
the
11-01-lint_has_been_silently_timing_out
branch
from
f9fa551
to
d4706f4
Compare
ndr-ds
changed the title
ma2bd
reviewed
Nov 5, 2024
.github/workflows/rust.yml
Outdated
| @@ -160,10 +160,9 @@ jobs: | |||
| cd linera-views | |||
| WASM_BINDGEN_TEST_TIMEOUT=300 wasm-pack test --chrome --headless -- --features web-default | |||
|
|
|||
| lint: | |||
| unexpected-chain-load-operations: | |||
There was a problem hiding this comment.
Should we keep lint somewhere in the name? Maybe lint-unexpected-chain-load-operations
There was a problem hiding this comment.
(even better) Should we group the fast lints together to save git-checkout time?
There was a problem hiding this comment.
Never mind "checkout" is surprisingly fast.
"Clear some space" is slow, though. In fact, we probably shouldn't do it for linters.
ma2bd
reviewed
Nov 5, 2024
.github/workflows/rust.yml
Outdated
| @@ -176,6 +175,19 @@ jobs: | |||
| - name: Check for unexpected chain load operations | |||
| run: | | |||
| ./scripts/check_chain_loads.sh | |||
|
|
|||
| check-copyright-headers: | |||
There was a problem hiding this comment.
nit: Maybe lint-check-copyright-headers. In fact lint-cargo-clippy is probably logical after that
ma2bd
reviewed
Nov 5, 2024
.github/workflows/rust.yml
Outdated
| @@ -227,12 +296,54 @@ jobs: | |||
| cargo rdme --check --no-fail-on-warnings -w "${dir_name}" | |||
| fi | |||
| done) | |||
|
|
|||
| wasm-application-lints: | |||
ma2bd
reviewed
Nov 5, 2024
ndr-ds
force-pushed
the
11-01-lint_has_been_silently_timing_out
branch
from
d4706f4
to
83d1692
Compare
ndr-ds
force-pushed
the
11-01-lint_has_been_silently_timing_out
branch
from
83d1692
to
792bc4c
Compare
ndr-ds
force-pushed
the
11-01-lint_has_been_silently_timing_out
branch
from
792bc4c
to
a5479f7
Compare
ma2bd
approved these changes
Nov 5, 2024
ndr-ds
pushed a commit
that referenced
this pull request
Nov 5, 2024
## Motivation CI takes a long time ## Proposal Split into more jobs, which will make us do roughly the same amount of work, but more parallelized. So hopefully this reduces CI time without increasing github costs much. ## Test Plan CI With this and the PR before this one (#2785), we went from having the `test` job sometimes take up to an hour, to now, running many jobs in parallel, the test that takes the longest is `check-all-features`, which takes around 30 minutes. ## Release Plan - Nothing to do / These changes follow the usual release cycle.
ma2bd
pushed a commit
that referenced
this pull request
Nov 11, 2024
On #1737 we added `continue-on-error` on the job level. AFAIU that will succeed even if the entire jobs fails with a cancellation for example, which will happen when we timeout. This was causing this job to silently succeed while timing out for a while. Split the different lints into their own jobs, adding parallelism and also giving us individual failures CI - Nothing to do / These changes follow the usual release cycle.
ma2bd
pushed a commit
that referenced
this pull request
Nov 11, 2024
## Motivation CI takes a long time ## Proposal Split into more jobs, which will make us do roughly the same amount of work, but more parallelized. So hopefully this reduces CI time without increasing github costs much. ## Test Plan CI With this and the PR before this one (#2785), we went from having the `test` job sometimes take up to an hour, to now, running many jobs in parallel, the test that takes the longest is `check-all-features`, which takes around 30 minutes. ## Release Plan - Nothing to do / These changes follow the usual release cycle.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
On #1737 we added
continue-on-erroron the job level. AFAIU that will succeed even if the entire jobs fails with a cancellation for example, which will happen when we timeout.This was causing this job to silently succeed while timing out for a while.
Proposal
Split the different lints into their own jobs, adding parallelism and also giving us individual failures
Test Plan
CI
Release Plan