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

Enable lints for discussion #356

Merged
merged 3 commits into from
Nov 20, 2023
Merged

Enable lints for discussion #356

merged 3 commits into from
Nov 20, 2023

Conversation

rkuris
Copy link
Collaborator

@rkuris rkuris commented Nov 18, 2023

These lints are only in fwdctl because it was easy to fix the few errors that popped up. Once we agree these are the right lints, we can move them to the top level.

For ease of review, here are links to the lint descriptions we are enabling as well reasons why I think they are important:

These lints are only in fwdctl because it was easy to fix the few errors
that popped up. Once we agree these are the right lints, we can move
them to the top level.

For ease of review, here are links to the lint descriptions we are
enabling as well reasons why I think they are important:

 - https://rust-lang.github.io/rust-clippy/rust-1.74.0/index.html#/unwrap_used
 - - code that panics should be avoided, except in tests
 - - not all panics have good error messages
 - https://rust-lang.github.io/rust-clippy/rust-1.74.0/index.html#/indexing_slicing
 - - panics
 - - there are good alternatives to index dereference that do not panic
 - https://rust-lang.github.io/rust-clippy/rust-1.74.0/index.html#/explicit_deref_methods
 - - easier and clearer just to dereference
 - https://rust-lang.github.io/rust-clippy/rust-1.74.0/index.html#/missing_const_for_fn
 - - non-const functions prevent callers from being const
@rkuris rkuris linked an issue Nov 18, 2023 that may be closed by this pull request
@rkuris rkuris merged commit 8c4bd64 into main Nov 20, 2023
5 checks passed
@rkuris rkuris deleted the rkuris/harden-clippy-fwdctl branch November 20, 2023 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants