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

Move ISAAC generators to their own crate #551

Merged
merged 1 commit into from Jul 16, 2018
Merged

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Jul 12, 2018

Part of #431

cargo doc --no-deps --all --all-features seems to fail in my tests unless I remove the --all-features flag. Not sure what's wrong here.

I left the PRNG comparisons in the prng module doc for now. We may decide to move that under doc/ later.

I also left the benchmarks as-is; this allows them to be compared across crates. I'm not sure whether to keep rand_isaac a dev-dependency just for this; perhaps eventually we should move the benchmarks.

For now the lib depends on rand_isaac anyway, just for the deprecation warnings (existing ones, and new ones for the names in the prng module).

@vks
Copy link
Collaborator

vks commented Jul 12, 2018

I think the ISAAC generators should be removed from the table as well. Other than that, it looks good to me.

@dhardy
Copy link
Member Author

dhardy commented Jul 12, 2018

I'd like to have more comprehensive overviews of available PRNGs. These ISAAC generators are not quite CSPRNGs but that was where they fit best.

Okay, I admit, part of this is just not wanting to throw away all the work @pitdicker did on these implementations. 😄

@dhardy dhardy added the D-review Do: needs review label Jul 12, 2018

## Crate Features

`rand_isaac` supports `no_std` and `alloc`-only configurations, as well as full
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This crate always works with no_std, and only has the serde1 feature?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, I forgot to update this bit. 👍

Copy link
Contributor

@pitdicker pitdicker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I admit, part of this is just not wanting to throw away all the work @pitdicker did on these implementations. smile

Git history preserves everything 😄. Still, having a good implementation of ISAAC in the ecosystem seems like good to have.

@dhardy
Copy link
Member Author

dhardy commented Jul 16, 2018

Git history preserves everything

Until you purge it! But discoverability and ease-of-use also matter I think.

@dhardy dhardy merged commit 1a98577 into rust-random:master Jul 16, 2018
@dhardy dhardy deleted the isaac branch July 16, 2018 10:11
@dhardy dhardy mentioned this pull request Sep 22, 2018
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-review Do: needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants