Use cargo clippy instead of the clippy plugin #176
#184
Conversation
Cargo.toml
Outdated
| @@ -15,7 +15,7 @@ travis-ci = { repository = "pingcap/rust-prometheus" } | |||
|
|
|||
| [features] | |||
| default = [] | |||
| dev = ["clippy"] | |||
| dev = [] | |||
There was a problem hiding this comment.
You can remove this feature now. :)
There was a problem hiding this comment.
Ok. is it safe to also remove these two lines on the Makefile? I had no idea since I had not use any Makefile yet😂
https://github.com/pingcap/rust-prometheus/blob/master/Makefile#L11
There was a problem hiding this comment.
You can remove the dev feature and make make dev to be format + test.
src/lib.rs
Outdated
| #![cfg_attr(not(feature = "dev"), allow(unknown_lints))] | ||
| #![cfg_attr(feature = "dev", allow(needless_pass_by_value))] | ||
| #![cfg_attr(feature = "dev", allow(new_without_default_derive))] | ||
| #![allow(unknown_lints)] |
There was a problem hiding this comment.
Can you combine these together?
There was a problem hiding this comment.
Previously we allow unknown_lints when feature is not dev. I guess for now this allow should be removed?
There was a problem hiding this comment.
done, by using
#![cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value, new_without_default_derive))]
| cargo test --features="$FEATURES $LINT"; | ||
| # Lint nightly feature. | ||
| cargo test --features="$FEATURES $LINT nightly"; | ||
| if [ $CLIPPY = 'yes' ]; then |
There was a problem hiding this comment.
Since CIPPY is always yes, can we remove this condition test?
There was a problem hiding this comment.
the CLIPPY env is only set yes when rust is nightly on the matrix section, so I thought the condition test is still need
also fix some rustfmt config
| @@ -1,5 +1,2 @@ | |||
| reorder_imports = true | |||
| reorder_imports_in_group = true | |||
There was a problem hiding this comment.
Why are these formatting rules removed?
There was a problem hiding this comment.
Oh, I see. These configurations are removed from rustfmt.
There was a problem hiding this comment.
seems these configurations are merged to reorder_imports, as the docs said for reorder_imports:
Reorder import and extern crate statements alphabetically
in groups (a group is separated by a newline).
|
LGTM |
Hi
first sorry for my messy commit message... I did not realized even I use force push to make the commit clean, but leaved the useless history in the issue page.
Currently the clippy result is same as using clippy plugin, except that even there is some clippy issues , the returned status is still 0, maybe related to rust-lang/rust-clippy#1209, so the CI will mark it as success.