-
Notifications
You must be signed in to change notification settings - Fork 431
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 explicit features rather than implicit. #1473
base: master
Are you sure you want to change the base?
Use explicit features rather than implicit. #1473
Conversation
d4a825d
to
9759584
Compare
It seems like every other crate offering a |
I think that would be good. I'd be happy to do that if there's agreement that it should happen. (I don't know how this project is governed.) |
IIRC, we were asked by someone (maybe @dtolnay?) back in the day to use |
IIRC it was my idea to use |
9759584
to
b745e18
Compare
I've opened #1477 which does the rename, if that's the way you decide to go. I would then rebase this PR once it landed to pick up the remaining explicit dependencies. |
Thanks. I'm really just waiting to see if other people have opinions on this. |
6d4e29d
to
718d7d4
Compare
In Rust 2024 edition, implicit features from optional dependencies are going away and explicit features are required. This changes all features using optional dependencies to use the "dep:" syntax to avoid creating implicit features. It also adds new explicit features for optional dependencies that didn't previously have an explicit feature to enable them. One user-visible implicit feature has now been removed: `rand` no longer has an implicit feature `rand_chacha` as that is enabled by `std_rng`.
718d7d4
to
2eed721
Compare
@dhardy This is now rebased on top of the rename of the |
@@ -30,6 +30,7 @@ alloc = ["rand/alloc"] | |||
std_math = ["num-traits/std"] | |||
|
|||
serde = ["dep:serde", "rand/serde"] | |||
serde_with = ["dep:serde_with"] |
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.
This doesn't show up either in API docs or the README. Could you add at least the latter please, preferably also a serialisation test?
Or we could just roll it under the serde
feature. Either is acceptable.
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.
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.
It was introduced for serializing const arrays, see the commit.
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.
Okay, so it should be just part of the serde
feature and not a separate (previously implicit) serde_with
feature.
I'll update this PR over the weekend for that.
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.
I agree!
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.
@waywardmonkeys a friendly reminder (if you'd be so kind)
CHANGELOG.md
entrySummary
In Rust 2024 edition, implicit features from optional dependencies are going away and explicit features are required.
Motivation
Preparing for the future, making features more clear when using
cargo add
.Details
This changes all features using optional dependencies to use the "dep:" syntax to avoid creating implicit features.
It also adds new explicit features for optional dependencies that didn't previously have an explicit feature to enable them.
This does mean that some previously publicly visible features are now gone, like
serde
. These features would not have worked previously as the code is guarded by, for example, checks forfeature = "serde1"
, so just enabling the optional dependency wouldn't have been sufficient to enable the code.