Improve CI, requring all feature combos to succeed along with clippy #321
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
Also fix new lint `noop_method_call`, which is present on the latest nightly compiler (rustc 1.74.0-nightly 2023-08-29)
I can't believe our CI hasn't caught this compilation error earlier.
Clarifies things & will reduce diff for future changes.
Unfortunately, it appears that actions-rs/toolchains is unmaintained. Switch to dtolnay/rust-toolchain which appears to be the new de-facto standard.
This version is important, because the wiki says it places an upper bound on the Minimum Supported Rust Version. In other words, it is the highest rust version we can use as MSRV. This new syntax is made possible by the switch to dtolnay/rust-toolchain.
This is only required for MSRV and hardcoded rust versions. It is possible that an upgrade to stable rust will introduce warnings that would break existing code. For this reason, we hardcode a recent version of stable rust to run clippy on in addition to the MSRV, stable, and nightly.
Before, we only required tests to succeed on the default features. This was necessary because we had CI disabled for a long time when Travis CI got rid of their free plans. It took us a while to switch to Github Actions and by then some code had broken on special feature combinations. Now, we require all code to pass tests by default. We can grant specific exceptions on a case-by-case basis. In particular, this may be needed for the Minimum Supported Rust Version.
The is because recent versions of erased-serde require edition="2021" which was stablized in 1.56. TODO: Add back testing suport for all feature combinations on MSRV. Should be possible once we bump MSRV to 1.56 (about two years old I think?) Could also consider adding cirrus-ci as a secondary CI since they have support for complex starlark configuration instead of YAML.
Techcable
force-pushed
the
fixup/2023-09-03/clippy
branch
2 times, most recently
from
d3dd4c0 to
f4d184b
Compare
This is the first version that supports edition="2021". It is a major release, and appears to be the earliest version that our (optional) dependency `erased-serde` supports.
Fix clippy lints for these example targets & test code.
This requires including the Minimum Supported Rust Version (MSRV). Additionally, document in Cargo.toml the different places where the MSRV needs to be kept in sync.
Techcable
force-pushed
the
fixup/2023-09-03/clippy
branch
from
f4d184b to
798a6ba
Compare
|
Hi @dpc , now that it is the weekend, could you maybe review this? 😃 |
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.
Significantly cleanup & improve our CI setup.
This requires clippy lints to succeed across two hardcoded versions (MSRV and something recent).
It also requires all feature combinations to pass the tests (with an exception for
nested-valueson MSRV).Big Changes
This is needed because the
erased-serdedependency requires edition="2021", added in Rust 1.56Minor Changes
Make sure to: