Extended CI suite #385
Extended CI suite #385
Conversation
Reorder features (by number of dependencies) Create CI profile, set MSRV Switch CI cache to Swatinem, reorder CI steps Swatinem is for starters just simpler to use and keys very sensitively. It also eliminates unnecessary files from `./target/`, which might be a good idea to cache with the now quite large number of jobs. I reordered the features also somewhat in user importance (increasing). The CI profile is primarily intended to reduce the size of the `./target/` folder. Reorder execution for speed/latency - clippy is fast.
| @@ -12,7 +12,33 @@ env: | |||
| RUST_FEATURES: "rodio-backend,lyric-finder,media-control,image,notify,clipboard" | |||
There was a problem hiding this comment.
I realize once you surround the arguments with quotes, spaces can be used instead of commas. See my below suggestions
There was a problem hiding this comment.
then you also need to update this variable to use spaces
| - os: ubuntu-latest | ||
| OS_SPECIFIC_FEATURES: ",daemon, sixel" | ||
| - os: macOS-latest | ||
| OS_SPECIFIC_FEATURES: ",sixel" |
There was a problem hiding this comment.
| - os: ubuntu-latest | |
| OS_SPECIFIC_FEATURES: ",daemon, sixel" | |
| - os: macOS-latest | |
| OS_SPECIFIC_FEATURES: ",sixel" | |
| - os: ubuntu-latest | |
| OS_SPECIFIC_FEATURES: "daemon sixel" | |
| - os: macOS-latest | |
| OS_SPECIFIC_FEATURES: "sixel" |
| msrv: | ||
| if: github.event.pull_request.draft != true | ||
| strategy: | ||
| fail-fast: false |
There was a problem hiding this comment.
I think that being a check rather than build or test, the job should be fast enough to not matter for a few seconds, and adds the feedback on whether it broke for every platform or not - likely would be a platform dependant dependency.
Though if keeping the MSRV for builds with all (compatible) features it would have to be bumped anyway.
| @@ -12,7 +12,33 @@ env: | |||
| RUST_FEATURES: "rodio-backend,lyric-finder,media-control,image,notify,clipboard" | |||
|
|
|||
| jobs: | |||
| rust-ci: | |||
| msrv: | |||
There was a problem hiding this comment.
can msrv be put as a step of rust-ci instead to avoid code duplication between those two?
There was a problem hiding this comment.
I'm seeing if the solutions in this discussion to reuse matrices can be applied in a simple way here. Notable possible problem is that they don't use the same matrices as it is right now.
That's your main concern I assume, though the checkout and install packages steps could be abstracted away into another workflow, too.
There was a problem hiding this comment.
I thought you only need to move
- name: Install cargo-hack
uses: taiki-e/install-action@cargo-hack
- name: Run MSRV check
run: cargo hack check --rust-version --package spotify_player --all-targets --profile ci --features "${{ env.RUST_FEATURES }} ${{ matrix.OS_SPECIFIC_FEATURES }}"
to be in rust-ci.steps.
There was a problem hiding this comment.
I originally wanted to to have them as separate jobs in order to report these separately - at the end of the day, if it doesn't build in the current compiler that's a hard error, but breaking MSRV is salvageable, if anything by bumping the MSRV.
But we can do that, we would just have to check the log if it does break
There was a problem hiding this comment.
yeah, one needs to check the action's logs if CI run fails anyway.
Co-authored-by: Thang Pham <[email protected]>
|
Alright I'm making this a draft - I'm trying to figure a way without repetitions but that keeps the jobs separate. As well as not being as complicated as that in that blasted discussion I linked earlier. I'm working on another (personal) branch - be back in a few days |
aome510
force-pushed
the
master
branch
2 times, most recently
from
6fc041a
to
15170ac
Compare
Took me a while to get around to it, but finally there's something to show for it.
./target/and creates a CI profile to reduce its cache usage, though Swatinem does a good job alreadycargo clippyandtestto have the quickest at the beginningI also took the liberty of reordering the features in the Cargo.toml file, in decreasing number of dependencies, mostly. We can drop that.
New (action) dependencies: taiki-e/install-action@v2 and Swatinem/rust-cache