Skip to content

Go clippy-free and a cleanup #96

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 26, 2021
Merged

Go clippy-free and a cleanup #96

merged 2 commits into from
Feb 26, 2021

Conversation

ojeda
Copy link
Member

@ojeda ojeda commented Feb 26, 2021

A couple clippy-related commits (see the commit messages)

ojeda added 2 commits February 26, 2021 05:31
@ojeda
Go clippy-clean for the rust/* crates and enforce it
302e275 
This covers the default clippy warnings.

Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda
Avoid passing rustc as second parameter to clippy-driver
2cd2b9a 
It turns out that is only a workaround intended for `cargo`.
The official instructions since PR [1] describe the usage of
`clippy-driver`, and they intentionally only cover replacing
`rustc` with `clippy-driver` rather than wrapping it, as
discussed in that PR.

[1] rust-lang/rust-clippy#6782

Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda
Copy link
Member Author

ojeda commented Feb 26, 2021

Before I forget: I am not sure whether to implement Default or disable the lint. My gut feeling was that the trait is intended mainly for POD-like types, even for things like String, but then I saw the standard library actually implements it even for things like RwLock too, so I guess it is fine?

alex
alex approved these changes Feb 26, 2021
View reviewed changes
Copy link
Member

@alex alex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also feel weird about the Default implementations -- neither of those types seems like places that really benefit from it. But also I don't feel super strongly and the rest of the changes look good.

@ojeda
Copy link
Member Author

ojeda commented Feb 26, 2021

Thanks! I guess since we both feel it is a bit weird we can take it out and disable the lint globally, since I bet we will hit this for many things... Anyway, not a big deal at this point.

@ojeda ojeda merged commit 0440a1c into rust Feb 26, 2021
@ojeda ojeda deleted the rust-clean-clippy branch February 26, 2021 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex alex alex approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ojeda @alex