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

Split JitterRng into a separate crate #685

Merged
merged 12 commits into from Jan 17, 2019
Merged

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Jan 7, 2019

@dhardy
Copy link
Member

dhardy commented Jan 7, 2019

Nice job but writing the impl before any decision is potentially a wasted effort. I appreciate the extra modularisation though.

Note if we do use this code: don't forget the copyright boilerplate this time (excepting the log shim which is trivial).

rand_jitter/README.md Outdated Show resolved Hide resolved
rand_jitter/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@vks vks left a comment

Choose a reason for hiding this comment

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

I think rand/utils/ci/script.sh still needs to be updated to include the new crate.

Looks good, besides the nits!

Copy link
Collaborator

@vks vks left a comment

Choose a reason for hiding this comment

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

Looks good to me now, thanks! Let's make sure to resolve the discussion in #681 before merging.

@dhardy
Copy link
Member

dhardy commented Jan 8, 2019

If we are going to do this, then I think it should be an optional dependency.

@newpavlov
Copy link
Member Author

Then you'll have to publish v0.7, as JitterRng export is not feature gated right now.

@dhardy
Copy link
Member

dhardy commented Jan 17, 2019

Summary from #681 and #699:

  • there are some reservations about JitterRng's security (JitterRng security #699)
  • we decided to stop using this as a direct dependency of Rand, though not to make breaking changes before 0.7 so this is still a dependency for now
  • when no longer a dependency, this will migrate to a new repository

@dhardy dhardy merged commit b09f92a into rust-random:master Jan 17, 2019
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.

None yet

3 participants