-
-
Notifications
You must be signed in to change notification settings - Fork 953
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: move utils to the correct place #2964
Conversation
✅ Deploy Preview for fakerjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2964 +/- ##
==========================================
- Coverage 99.95% 99.95% -0.01%
==========================================
Files 2984 2984
Lines 216057 216057
Branches 597 595 -2
==========================================
- Hits 215969 215957 -12
- Misses 88 100 +12
|
@watercubz Please also add a comment to the issue so that we can assign it to you for better visibility. |
src/internal/mersenne.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also move the
faker/src/internal/mersenne.ts
Line 333 in 05237e4
export function generateMersenne32Randomizer(): Randomizer { |
And the 53bit variant to src/utils/mersenne.ts
I think |
Team Decision
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (blocking): MersenneTwister19937 is an internal class and should stay in internal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarification: the mersenne class is internal, the generateXRandomizer utility functions are public.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, then mersenne goes internal, and randomizer would go outside of utils and types in internal along with mersenne, would that be the correction to the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this answer your question:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course I think I already understood the point, thank you very much, I will upload changes to those previously mentioned in a moment.
@watercubz is this ready for review again? You mentioned in #2964 (comment) that you want to upload something, but I cannot see further commits from there. Maybe you missed something? If it is ready for review again, please click the "Ready for review" button so we get notified. |
Superseded by #3141. |
Issue #2961: Move Functions and Types to Proper Directories
Changes Made
generateMersenne*Randomizers()
functions to thesrc/utils
directory.src/utils/types.ts
to thesrc/internal
directory.Additional Details
Screenshots
Attached screenshots showing the new directory structure and updated imports.




Related Issue