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.

Sign up for GitHub

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

Jump to bottom

[2487] Fixed random selection issue #2488

Merged
merged 1 commit into from
May 15, 2022
Merged

[2487] Fixed random selection issue #2488

merged 1 commit into from
May 15, 2022

Conversation

sudeeptarlekar
Copy link
Contributor

Issue#

closes #2487

Description:

Fixed random class issue.

@sudeeptarlekar
[2487] Fixed random selection issue
04d2703 
Signed-off-by: sudeeptarlekar <sudeeptarlekar@gmail.com>
connorshea
connorshea approved these changes May 15, 2022
View reviewed changes
Copy link
Member

@connorshea connorshea left a comment

Choose a reason for hiding this comment

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

I'm surprised this works on all Ruby versions honestly, but the tests pass so ¯_(ツ)_/¯

@connorshea
Copy link
Member

I tried this locally and can confirm 1) this is an actual problem, and 2) it is resolved by this change.

@connorshea connorshea merged commit 7fa13f2 into faker-ruby:master May 15, 2022
@connorshea connorshea mentioned this pull request May 15, 2022
Closed
@Qqwy
Copy link

Qqwy commented May 11, 2023

I'm surprised this works on all Ruby versions honestly, but the tests pass so ¯_(ツ)_/¯

As long as only .rand is used then it will work on all Ruby versions, since Random::rand has been available (delegating to Random::DEFAULT.rand) on old Ruby versions. But the same is not true for .seed. Currently the only place where .seed seems to be used is in the README so this is probably fine.

@tmr111116 tmr111116 mentioned this pull request Jul 1, 2023
Merged
@sudeeptarlekar sudeeptarlekar deleted the 2487-random-bug branch December 1, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@connorshea connorshea connorshea approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change of Random::DEFAULT -> Random.new is incorrect
3 participants
@sudeeptarlekar @connorshea @Qqwy