Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(s2n-quic-transport): fix clippy beta warnings #1390

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Jul 6, 2022

Description of changes:

Clippy introduced a new lint in beta that checks to see if Eq can be derived if PartialEq is also derived. This is fine for applications but it makes ensuring backward-compatibility in libraries more fragile. If a field is added that doesn't implement Eq, then the derive would have to be removed, which could break existing callers.

This change adds an allow for the new lint globally, since I don't think it's very compelling.

See rust-lang/rust-clippy#9063

I've also made beta clippy errors into warnings since it clutters up actual statuses every time clippy adds one of these things.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@camshaft camshaft force-pushed the camshaft/clippy-beta-07-2022 branch 2 times, most recently from b5d70a0 to bb9964f Compare July 11, 2022 19:26
@camshaft camshaft force-pushed the camshaft/clippy-beta-07-2022 branch from bb9964f to d8ae1ea Compare July 11, 2022 19:30
@camshaft camshaft marked this pull request as ready for review July 11, 2022 19:54
@toidiu
Copy link
Contributor

toidiu commented Jul 11, 2022

can we do this in a clippy.toml so that it also applies to the local_test script?

@camshaft
Copy link
Contributor Author

can we do this in a clippy.toml so that it also applies to the local_test script?

I wish we could... but since this lint is unstable it needs to be applied conditionally depending on the clippy version. And to complicate things even more: they've deprecated unknown_clippy_lints (see rust-lang/rust-clippy#6653), which makes it difficult to make our allow attributes compatible with our MSRV

@camshaft camshaft enabled auto-merge (squash) July 11, 2022 21:43
@camshaft camshaft disabled auto-merge July 12, 2022 17:28
@camshaft camshaft merged commit f9f49dd into main Jul 12, 2022
@camshaft camshaft deleted the camshaft/clippy-beta-07-2022 branch July 12, 2022 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants