-
Notifications
You must be signed in to change notification settings - Fork 163
Format fix document etc #389
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.
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
Conversation
…//rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods] It's what the reader would be expecting.
…sion that something is being returned: [https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned]
…t closures that trivially wrap the methods: [https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls] Clippy says it's better to call default methods of the struct, rather than the trait: [https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access]
…they are better expressed as associated functions.
…ntry data, so it is better as a borrowed parameter.
…d be labeled by a markdown section `#` in the doc string. [https://rust-lang.github.io/rust-clippy/rust-1.51.0/index.html#missing_errors_doc] I've done my best to write some kind of reasonable statement; there is room for improvement.
…ault implementations may or may not use the variables, but that doesn't help the reader understand the API; it is merely distracting.
…iends were removed.
…, `Lseek`). In `lib.rs`, moved the `ReplyX` to their own seperate lines; it's the only way to make cargo fmt alphebetize them properly. In `reply.rs`, no longer importing individual items from `::ll`; it was too hard to track them all.
…, because it's merely a coincidence that they only appear under certain feature-gated blocks; they aren't actually related to those feature gates.
…ctory commands on non-directory targets.
…sion that something is being returned: [https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned]
…ersions (e.g., `abi-7-36`).
…onstant on a non-linux OS.
… and future reorganization.
…ause the target file doesn't exist. It can still fail, but the test failure should be related to the feature being tested (i.e., passthrough).
…after calling background session `join`.
…e some testing on macOS (for example, I did, and then fixed some bugs).
…ture testing and reorganization easier.
…on would not work correctly for certain abi versions (e.g., less than `abi-7-21`).
…rganization easier.
…on. Seperated testing for `opened` and `opened_passthrough`.
… but not a future.
…best to explain by comment why each clippy allow attribute is appropriate.
…ave up fighting it.
|
Oh great! Ya, would be nice to have stricter Clippy checking. Can you submit this in a series of smaller PRs? Let's aim for ~100 lines each. That'll make it easier for me to review |
|
Could you please clarify how you envision I would construct a collection of pull requests? Are these going to be in parallel or in series, all at once or one at a time? |
|
ya, I was thinking one at a time, in series. Like just take the first few commits from this one to start, and then as soon as that's merged the next few |
|
This pull request has been superseded by a series starting with #391 |
What I really want to do is submit my big refactor PR, but I can't resist making all these small fixes on the way. But it's not fair to the reviewer to review many small changes and a big refactoring change at the same time. So I decided to fix as much as I can, up front. I am proud to present this branch, in which, to my knowledge, for the first time ever, cargo clippy is totally satisfied.