Fix/pre commit and clippy - #257
Conversation
Fix warnings: - redundant reference in `print!` argument - this can be `std::io::Error::other(_)`
cpu
left a comment
cpu
left a comment
There was a problem hiding this comment.
Seems OK if you like pre-commit hooks :-)
I had some nits to consider but nothing I consider blocking. Thanks!
| hex-literal = "0.4" | ||
|
|
||
| [lints.clippy] | ||
| #pedantic = { level = "warn", priority = -1 } |
There was a problem hiding this comment.
Why land commented-out config? I think it'd be tidier to remove this and the other print_stdout/print_stderr entries if they're not being used.
There was a problem hiding this comment.
the objective is to un-comment them progressively. If we activate the warnings right now, there are a lot of fixes
There was a problem hiding this comment.
I understand not wanting to activate it immediately, but I don't know that I see the value in leaving it commented out as some kind of reminder. Wouldn't a GitHub issue be better for tracking the work required to enable pedantic clippy if that's a goal?
|
|
||
| [lints.rust] | ||
| unsafe_code = "forbid" | ||
| #missing_docs = "warn" |
There was a problem hiding this comment.
same feedback to consider here RE: not adding commented out config.
| uses: rusticata/ci-action-rdme@v1 | ||
| uses: rusticata/ci-action-rdme@v2 | ||
| with: | ||
| rdme-args: --intralinks-features validate,verify,verify-aws |
There was a problem hiding this comment.
It'd make for a tidier commit history if this were folded into the commit that added the pre-commit config 👼
There was a problem hiding this comment.
It can be done, I just thought of these commits as slightly different (one for pre-commit and one for rdme) which could have been independant
|
|
||
| assert!(rem.is_empty()); | ||
| dbg!(csr.certification_request_info.attributes()); | ||
| //dbg!(csr.certification_request_info.attributes()); |
There was a problem hiding this comment.
I would vote to remove this line instead of commenting it out.
Thanks for the review, I will push some updates then merge. |
2 participants
QA+CI improvements:
prekwhich is compatible but faster) to run hooks locally on commitsCargo.tomland enable more warnings