-
Notifications
You must be signed in to change notification settings - Fork 29
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
use 2018 edition idioms #149
Conversation
this would add 2018 edition idioms just in time for the 2021 edition... the only place this is likely to bite even slightly is the 'extern crate' macro imports. there's a decent chance this style of import will become a compiler warning in 2021 edition (see rust-lang/rust#80165). Not a hard error, mind. Although keeping the 2015 idioms isn't actually going to cause any real problems, i'm inclined to believe the 2018 idioms were introduced for good reasons.
|
aa205ce
to
ea276e0
Compare
rebased on master |
The commit dropping the I think the explicit imports should be collapsed into one commit, since they're doing the same thing. |
I had assumed this would all be applied as a single squash merge |
The repo appears to be configured for squash merging by default (don't know why), but the preferred method is rebase merging. |
ea276e0
to
5cc4b33
Compare
i'm tempted to rebase this as a single commit (which is a squash merge) unless you particularly want these commits separated? |
I think how you did it in system76-power is good. One commit for moving files for 2018-style crates, and one commit for explicit imports. There's some rustfmt changes mixed in, which could also be a separate commit (or remove them and leave it for #155). |
5cc4b33
to
af5d47b
Compare
af5d47b
to
0129084
Compare
i've rebased and tidied the commits. i've left the formatting, for the simple reason that it would be painful to change. #155 can prevent this kind of noise happening again. Don't really mind which gets merged first, i can rebase either pull request on the other without too much pain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No regressions found.
this refactors the crate to use 2018 edition idioms
cargo +nightly fmt
)