Minor cleanup, resolve some Clippy lints #155
Conversation
|
Looks good. I like the inline format args and the edition suggestions, could you add them to the PR? |
Implicit elision of type lifetime parameters is deprecated, as per the rust-2018-idioms warning.
Clippy's needless_pass_by_value lint shows a couple functions that don't need to own their arguments. undeploy() in particular takes a 240-byte Options struct by value but, like deploy(), doesn't need mutable access.
Clippy's unnested-or-patterns lint caught this.
Clippy's match-same-arms lint caught this. The previous two functions already used or-patterns like this, but condition() was overlooked.
Clippy's unused-self lint caught this.
Clippy's doc-markdown lint caught this.
Due to being in a move closure, this clone captures opt by value and then clones it. This wouldn't work even if opt were used later in the enclosing scope and serves no purpose as it is.
Clippy's uninlined-format-args lint caught this.
Clippy's redundant-else lint caught this.
Rustfmt wants this broken due to line length and requires {} for
multiline match arms, but doesn't insert a ; because it only sees
syntax, not types. Clippy then complains (when pedantic) about the
implicit () return.
Clippy's clippy::semicolon-if-nothing-returned lint caught this.
Clippy's redundant-closure lint caught this.
Replaces `assert_eq!(x, [true|false])` with `assert!([x|!x])`. Clippy's bool-assert-comparison lint caught this.
When testing without the `scripting` feature, compilation would fail because a `Configuration` was constructed assuming the `helpers` field always exists, but it only exists when `scripting` is enabled.
In `deploy::test::{low_level_simple, low_level_skip}`,
`Default::default()` was used to construct a `BTreeMap`, with the only
indication of the variable's type being the signature of
`RealActionRunner::new()`, which it was passed by reference to.
Clippy's default-trait-access lint caught this.
|
I think I wrongly assumed you would be notified when I pushed commits, so this has sat open for longer than I expected. Sorry about that. Anyway, I've revisited this, rebased to resolve the minor conflict in This doesn't quite resolve everything that Clippy complains about with |
|
Looks great :D I commented on the Then it's ready for merge |
This is less verbose and provides a more detailed panic message.
`config::tests` was the only test module not named `test`.
|
I actually couldn't find any other uses of And, since I happened to notice, for good measure I also renamed the |
|
THanks for the contribution :D |
I ran
cargo clippy -- -W clippy::pedanticand fixed the issues it complained about that I thought were most reasonable. I also removed a redundant clone fromwatch.rswhich Clippy surprisingly didn't complain about.There were a few lints I decided against fixing, such as a
|p| p.render()closure inhandlebars_helpers.rswhich would need to be expanded to the fully-qualified pathhandlebars::PathAndJson::render. There were a lot of instances ofclippy::uninlined-format-args(fmt!("{} {}", x, y)→fmt!("{x} {y}")) which I didn't bother with, though if you prefer the inline style, you could runcargo clippy --fix -- -A clippy::all -W clippy::uninlined-format-argsto auto-fix them.I also noticed the crate is on
edition = "2018". Changing this toedition = "2021"doesn't seem to require any other changes to the code, so it should be an easy change if you want to do that at some point.