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 rand distr #1577

Merged
merged 8 commits into from
Feb 6, 2025
Merged

Move rand distr #1577

merged 8 commits into from
Feb 6, 2025

Conversation

benjamin-lieser
Copy link
Member

  • Added a CHANGELOG.md entry

Summary

#1394

Details

It removes all of the code which now lives in the rand_distr repository

@benjamin-lieser
Copy link
Member Author

I guess removing rand_distr has caused the minimal version of serde_derive to go from 1.0.157 to 1.0.103 which causes the CI failure

Copy link
Member

@dhardy dhardy 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.

The minimal-dependency-version test isn't much use then, is it? But some people want it (unless it's fallen out of fashion; I haven't heard much about this for a while).

benches/Cargo.toml Outdated Show resolved Hide resolved
@benjamin-lieser
Copy link
Member Author

Looks good.

The minimal-dependency-version test isn't much use then, is it? But some people want it (unless it's fallen out of fashion; I haven't heard much about this for a while).

Should we require a newer version of serde, if 1.0.103 is not working correctly?

@benjamin-lieser
Copy link
Member Author

Do we need utils/redirect.html? The ziggurat tables should probably only be in rand_distr.

@benjamin-lieser
Copy link
Member Author

Should there be a changelog entry? If yes where?

@benjamin-lieser benjamin-lieser marked this pull request as ready for review February 5, 2025 19:50
@benjamin-lieser
Copy link
Member Author

Looks good.

The minimal-dependency-version test isn't much use then, is it? But some people want it (unless it's fallen out of fashion; I haven't heard much about this for a while).

It was actually serde_json which is only a dev dependency. So I have bumped the version.

I think it is still sensible to check if the minimal version requirements work. For us having different crates in the same workspace leads to false negatives on it as we have seen here.

@dhardy
Copy link
Member

dhardy commented Feb 5, 2025

Do we need utils/redirect.html?

Yes (it's used by gh-pages.yml).

Should there be a changelog entry? If yes where?

No; it's not a change to any crate.

Copy link
Member

@dhardy dhardy 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.

The ziggurat tables should probably only be in rand_distr.

I think it's just this to do.

@dhardy dhardy merged commit 0bc3f65 into rust-random:master Feb 6, 2025
15 checks passed
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