Skip to content
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

Remove activation of rand_core/std #458

Closed

Conversation

tarcieri
Copy link
Contributor

@tarcieri tarcieri commented Dec 8, 2022

This feature provides two things:

  1. Transitive activation of the rand_core/getrandom feature
  2. Impl of std::error::Error for rand_core::Error

rand_core::Error is not part of curve25519-dalek's public API and is irrelevant.

The rand_core/getrandom feature provides OsRng which is used in tests only, so we can activate it in dev-dependencies.

This feature provides two things:

1. Transitive activation of the `rand_core/getrandom` feature
2. Impl of `std::error::Error` for `rand_core::Error`

`rand_core::Error` is not part of `curve25519-dalek`'s public API and is
irrelevant.

The `rand_core/getrandom` feature provides `OsRng` which is used in
tests only, so we can activate it in `dev-dependencies`.
@tarcieri tarcieri requested a review from rozbb December 8, 2022 18:00
@tarcieri tarcieri mentioned this pull request Dec 8, 2022
@rozbb
Copy link
Contributor

rozbb commented Dec 8, 2022

rand_core/std does one thing: impl<R: RngCore> RngCore for Box<R>. Theoretically a user might care about this trait.

But if we remove rand_core/std anyways, then we can remove std entirely, since subtle/std does nothing.

@rozbb
Copy link
Contributor

rozbb commented Dec 8, 2022

Actually that's wrong. The README says that, but in reality you only need alloc. So I guess we can remove std altogether

@tarcieri
Copy link
Contributor Author

tarcieri commented Dec 8, 2022

Oh hey, you're right. Let me open a PR based on this one that removes the std feature entirely

Edit: opened #459

@tarcieri tarcieri closed this Dec 8, 2022
@tarcieri tarcieri deleted the remove-activation-of-rand_core-std-feature branch December 8, 2022 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants