-
Notifications
You must be signed in to change notification settings - Fork 204
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
MSRV bump, module changes #15
Conversation
- rust: 1.28.0 | ||
env: DESCRIPTION="OSX, 1.22.0" | ||
- rust: 1.32.0 | ||
env: DESCRIPTION="OSX, 1.32.0" |
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.
Is there any reason not to use 1.31 as the MSRV? Also you need to update the note in the README.
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.
extern crate std;
and use std::<..>;
in modules does not work on 1.31.0. It fails with "error[E0658]: imports can only refer to extern crate names passed with --extern
on stable channel (see issue #53130)". I think we could fix it by moving extern crate std;
to lib.rs
, but it will add a sizable cfg
attribute.
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.
Fair enough. 1.32 is barely over two months old, but there doesn't appear to be much demand for back compatibility. Perhaps we should not do the same for the main Rand project yet though just in case we need to revert this.
Can we use Edition 2018 style macro imports? |
I looks like |
Seems like your workaround does the job for now. It may require fixing again later, but both This is ready for merging then? |
Yes, it's ready to be merged. Regarding WASM, I think we should understand how future changes will be handled. Say we publish v0.1 and then |
I think if both As for our implementation, if we choose to deprecate one, we can simply add a |
For the same reasons we started doing it for the main app: coreos#1719 This time, it's `getrand` that broke us. rust-random/getrandom#15 We should be able to update to 1.35.0 soon, which will unblock this.
For the same reasons we started doing it for the main app: coreos#1719 This time, it's `getrand` that broke us. rust-random/getrandom#15 We should be able to update to 1.35.0 soon, which will unblock this.
For the same reasons we started doing it for the main app: #1719 This time, it's `getrand` that broke us. rust-random/getrandom#15 We should be able to update to 1.35.0 soon, which will unblock this. Closes: #1865 Approved by: cgwalters
For the same reasons we started doing it for the main app: #1719 This time, it's `getrand` that broke us. rust-random/getrandom#15 We should be able to update to 1.35.0 soon, which will unblock this. Closes: #1865 Approved by: cgwalters
Note that with this PR
getrandom
by default will not use anything fromstd
anymore, so it will be possible to implement #4.