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

chore: Add clippy to ci #1189

Merged
merged 8 commits into from
Dec 14, 2022
Merged

Conversation

tottoto
Copy link
Collaborator

@tottoto tottoto commented Dec 13, 2022

Adds clippy check to ci.

@tottoto tottoto marked this pull request as draft December 13, 2022 19:44
@tottoto tottoto marked this pull request as ready for review December 13, 2022 20:01
@LucioFranco
Copy link
Member

I am almost more inclined to not include clippy in ci but to accept clippy PRs as them come. Mostly this is due to my frustrations of new versions of clippy adding new lints such that it blocks PRs that add unrelated things.

@tottoto
Copy link
Collaborator Author

tottoto commented Dec 13, 2022

That makes sense. I understand your point and agree with you. So how do you think about lock the clippy version to the msrv for minimal lint (if it is feasable). Is it acceptable?

@tottoto
Copy link
Collaborator Author

tottoto commented Dec 13, 2022

I'll try to implement it.

@LucioFranco
Copy link
Member

Yeah, I am ok to lock the clippy version so that new code get caught but old code doesn't cause issues when a new version comes out.

@LucioFranco
Copy link
Member

on a sidenote are you on discord in the tokio server?

@tottoto
Copy link
Collaborator Author

tottoto commented Dec 13, 2022

I have to fix this warning.

warning: unknown lint: `clippy::derive_partial_eq_without_eq`
 --> tests/root-crate-path/src/main.rs:1:9
  |
1 | #[allow(clippy::derive_partial_eq_without_eq)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unknown_lints)]` on by default

@tottoto
Copy link
Collaborator Author

tottoto commented Dec 13, 2022

Ignores unknown lints and makes job fail when clippy reports warnings.

@tottoto
Copy link
Collaborator Author

tottoto commented Dec 13, 2022

Done. I think this is ready to be reviewed.

@tottoto
Copy link
Collaborator Author

tottoto commented Dec 13, 2022

on a sidenote are you on discord in the tokio server?

I'm not on there.

@LucioFranco LucioFranco merged commit 4e5c0b2 into hyperium:master Dec 14, 2022
@tottoto tottoto deleted the add-clippy-to-ci branch December 14, 2022 14:47
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.

None yet

2 participants