fix: update MSRV references and clippy lints #1671
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
jmayclin
force-pushed
the
fix-clippy-lint
branch
from
3269200
to
9ab9afd
Compare
jmayclin
changed the title
* Cargo.toml msrv specifications should match rust toolchain * derive_hash clippy lint naming update * fix deprecated openssl builder function
jmayclin
force-pushed
the
fix-clippy-lint
branch
from
9ab9afd
to
1d5e17f
Compare
jmayclin
changed the title
toidiu
previously approved these changes
Mar 21, 2023
| @@ -18,7 +18,7 @@ pub const LEN: usize = 128 / 8; | |||
| // The implemented PartialEq will have the same results as | |||
| // a derived version, except it is constant-time. Therefore | |||
| // Hash can still be derived. | |||
| #[allow(clippy::derive_hash_xor_eq)] | |||
There was a problem hiding this comment.
What if you did something like:
#[allow(unknown_lints, clippy::derive_hash_xor_eq, clippy::derived_hash_with_manual_eq)]
There was a problem hiding this comment.
Just tested it and that does allow us to continue using clippy 1.63. It's not super pretty, but probably better than making backwards incompatible clippy changes?
There was a problem hiding this comment.
Actually, I'm not sure what I missed when testing locally, but clippy still warns about the old name, (because it does recognize it) and then we error out on that.
There was a problem hiding this comment.
seems that trick used to work:
s2n-quic/common/s2n-codec/src/decoder/value.rs
Lines 104 to 109 in aec1ff9
but they deprecated
clippy::unknown_clippy_lints so I guess no longer
This reverts commit 4ffe886.
WesleyRosenblum
approved these changes
Mar 21, 2023
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.
Description of changes:
This PR is primarily cut to fix clippys warnings (that are transformed into an error by
-D warnings) in our CI run of clippy.the
hash_xor_partial_equallint was renamed toderived_hash_with_manual_eq. Note that allowingderived_hash_with_manual_eqis a backwards incompatible change for clippy. E.g. you will no longer be able to run clippy on the MSRV version. The alternative is switching our CI clippy runs to either use the MSRV or no longer treat all warnings as errors.Clippy also introduced a new complaint in stable (1.68) which is the manually derived Default on enums. This PR updates code to comply with that lint.
It also updates netbench MSRV to 1.62. Since netbench consumes quic and quic has an MSRV of 1.63, it couldn't actually be built with 1.62 anyways.
Call-outs:
We specify a patch version dependency for OpenSSL in netbench because the
buildfunction was deprecated in 010.46, but thebuild2function didn't exist before 0.10.46. So if we use the build function, then we require at least0.10.46to build.It does not fix the
derived_hash_with_manual_eqnaming error, because that lint wasn't available until 1.68. Ideally the clippy MSRV configuration would take care of that for us, but that does not appear to be the case.Testing:
CI run should pass successfully.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.