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

Add cargo clippy checks #22

Closed
morenol opened this issue Nov 5, 2020 · 4 comments · Fixed by #36
Closed

Add cargo clippy checks #22

morenol opened this issue Nov 5, 2020 · 4 comments · Fixed by #36
Assignees

Comments

@morenol
Copy link
Collaborator

morenol commented Nov 5, 2020

Fix cargo clippy warnings and add to the CI something that checks for that.

There is a cargo clippy --fix command that can solve some of the warnings, that could be used at the beginning.

The instructions to run cargo clippy are:

rustup component add clippy # to install it
cargo clippy
@VolodymyrOrlov
Copy link
Collaborator

@morenol I am going to close this issue since you have covered most basic checks. Let me know if you want to keep it open.

@morenol
Copy link
Collaborator Author

morenol commented Dec 11, 2020

These are the warning that we are ignoring right now in lib.rs

  • clippy::needless_range_loop,
  • clippy::ptr_arg,
  • clippy::type_complexity,
  • clippy::too_many_arguments,
  • clippy::many_single_char_names

What do you think about them? Maybe we can move that from lib.rs to the specific places where we have those lint errors and while doing that we can fix some of them, so far I have a branch all the places where clippy::needless_range_loop is happening but not sure if we should fix them or just ignore them.

@VolodymyrOrlov
Copy link
Collaborator

I think ignoring these warning is fine for now. From my point of view a lot of these problems can be ignored, like many_single_char_names or too_many_arguments since these problems will not lead to ineffective code or runtime crash.
In my opinion, only needless_range_loop and ptr_arg deserve to be fixed.

@morenol
Copy link
Collaborator Author

morenol commented Dec 11, 2020

perfect, then I will fix them. I also fixed some of the places where ptr_arg was appearing, I will create a PR with that

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 a pull request may close this issue.

2 participants