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

Remove usage of mt_rand #87

Merged
merged 5 commits into from
Dec 4, 2020
Merged

Remove usage of mt_rand #87

merged 5 commits into from
Dec 4, 2020

Conversation

IonBazan
Copy link

@IonBazan IonBazan commented Nov 30, 2020 •
edited
Loading

This PR

  • removes usage of mt_rand where it's not necessary and uses static::numberBetween() or
  • mt_rand(0, 1) usages were replaced with Miscellaneous::boolean()
  • Random dates generation (mostly for national IDs) were replaced by DateTime::dateTimeThisCentury() for consistency.

@IonBazan IonBazan force-pushed the feature/remove-mt_rand branch 2 times, most recently from 08a28ff to b80d744 Compare November 30, 2020 12:31
bram-pkg
bram-pkg requested changes Nov 30, 2020
View reviewed changes
src/Faker/ORM/CakePHP/ColumnTypeGuesser.php Outdated Show resolved Hide resolved
src/Faker/ORM/Doctrine/ColumnTypeGuesser.php Outdated Show resolved Hide resolved
bram-pkg
bram-pkg approved these changes Nov 30, 2020
View reviewed changes
Copy link
Member

@bram-pkg bram-pkg 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.

@pimjansen pimjansen added the enhancement New feature or request label Nov 30, 2020
@bram-pkg bram-pkg marked this pull request as draft December 1, 2020 19:11
@bram-pkg bram-pkg added the invalid This doesn't seem right label Dec 1, 2020
@bram-pkg bram-pkg changed the title avoid mt_rand usage Remove usage of mt_rand Dec 1, 2020
@IonBazan IonBazan marked this pull request as ready for review December 2, 2020 03:23
@IonBazan IonBazan force-pushed the feature/remove-mt_rand branch from 70ed24a to 79a738a Compare December 2, 2020 03:23
GrahamCampbell
GrahamCampbell requested changes Dec 2, 2020
View reviewed changes
src/Faker/Provider/Lorem.php Outdated Show resolved Hide resolved
@GrahamCampbell GrahamCampbell merged commit d26ac52 into FakerPHP:main Dec 4, 2020
@GrahamCampbell
Copy link
Member

GrahamCampbell commented Dec 4, 2020 •
edited
Loading

Excellent, thanks. Note that we may need to revert this if people were seeding the mt_ functions in their tests, and start complaining. I think this is unlikely, though. I'm sure anyone affected will let us know on the issue tracker, once the next release goes out.

@IonBazan IonBazan deleted the feature/remove-mt_rand branch December 4, 2020 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bram-pkg bram-pkg bram-pkg approved these changes

@GrahamCampbell GrahamCampbell GrahamCampbell requested changes

Assignees
No one assigned
Labels
enhancement New feature or request invalid This doesn't seem right
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@IonBazan @GrahamCampbell @bram-pkg @pimjansen