Some minor code linting #10
Conversation
nyurik
force-pushed
the
lints
branch
2 times, most recently
from
8d52dd0
to
88d56f8
Compare
| @@ -14,14 +14,25 @@ readme = "README.md" | |||
| repository = "https://github.com/static-files-rs/static-files" | |||
|
|
|||
| [features] | |||
| default = [ "change-detection" ] | |||
| default = ["change-detection"] | |||
| change-detection = ["dep:change-detection"] | |||
There was a problem hiding this comment.
Would it not be the same, as before?
There was a problem hiding this comment.
@kilork correct - i did not introduce any change here - instead I made it explicit that yes, we want the feature to be named the same as cargo dependency. There was some discussion how features and dependencies are intermingled, so having an implicit feature thing is less "rusty". But up to you of course.
Cargo.toml
Outdated
|
|
||
| [dependencies] | ||
| change-detection = { version = "1.2", optional = true } | ||
| mime_guess = "2.0" | ||
| path-slash = "0.1" | ||
| path-slash = "0.x" |
There was a problem hiding this comment.
0.x? Is it a typo, or just some cargo feature? :)
There was a problem hiding this comment.
This one is tricky. It compiles (your CI requires approval, so I ran it on my fork too), and it is recommended by the crate author, but it is not documented anywhere. The semver parser does understand it as a wildcard. I filed a bug dtolnay/semver#322
I will use 0.2 to avoid questions
| } | ||
| ``` | ||
| */ | ||
| #![doc = include_str!("../README.md")] |
There was a problem hiding this comment.
We should check, is it not affecting test runs.
There was a problem hiding this comment.
this code is identical to what you had before - you just had the content of the readme copy/pasted, whereas I let compiler copy/paste it instead. I tested it with cargo test, but then i noticed you used #ignore -- this is not good because it turns out you had a bug in that code (incorrect return value). I changed it to no_run -- this way both tests now compile, but do not execute
| writeln!( | ||
| f, | ||
| "{}.insert({:?},n(i!({:?}),{:?},{:?}));", | ||
| variable_name, &key_path, &abs_path, modified, &mime_type, | ||
| "{variable_name}.insert({key_path:?},n(i!({abs_path:?}),{modified:?},{mime_type:?}));", |
There was a problem hiding this comment.
If we use {qqq} we better set rust-version
There was a problem hiding this comment.
I ran cargo msrv on it, and added. thx.
nyurik
force-pushed
the
lints
branch
2 times, most recently
from
72ed9ff
to
4d71703
Compare
I tried to do some basic cleanups, but if you feel any of them are not relevant, let me know and I will revert those. * Make CI compile on PRs and on main merge, but not on all other pushes - makes it easier to contribute * Remove duplicate readme documentation * Migrate to 2021 edition * Make `change-detection` feature explicit, and use the `dep:` format * Add lints to the Cargo.toml * Upgrade dependencies * Inline format var use * Remove unneeded `&` refs * Add a few more tests to CI
|
P.S. I had to bump crate version because it was failing semver check |
I tried to do some basic cleanups, but if you feel any of them are not relevant, let me know and I will revert those.
change-detectionfeature explicit, and use thedep:format&refs