Skip to content

docs: fix mersenne rand min max confusion #839

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

Merged
merged 2 commits into from
May 23, 2022

Conversation

gregoriomario
Copy link
Contributor

although mersenne automatically swap the value if min is bigger than max, but it's a good practice to clear out the confusion.

Fix #775

@gregoriomario gregoriomario requested a review from a team as a code owner April 15, 2022 17:32
@ejcheng ejcheng added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Apr 15, 2022
@ejcheng ejcheng added this to the v6 - Non-Breaking Changes milestone Apr 15, 2022
@codecov
Copy link

codecov bot commented Apr 15, 2022

Codecov Report

Merging #839 (d6d3465) into main (2a6003f) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #839   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files        2110     2110           
  Lines      225732   225732           
  Branches      979      979           
=======================================
  Hits       224992   224992           
  Misses        720      720           
  Partials       20       20           
Impacted Files Coverage Δ
src/modules/mersenne/index.ts 100.00% <100.00%> (ø)

@ejcheng ejcheng requested a review from a team April 15, 2022 20:39
@ejcheng ejcheng changed the title docs: fix mersenne rand mix max confusion docs: fix mersenne rand min max confusion Apr 15, 2022
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I assume it was implemented with max as first param, so that a user can use it with faker.mersenne.rand(6) so without a min argument and it just has a default min = 0

gregoriomario and others added 2 commits May 23, 2022 19:46
although mersenne automatically swap the value if min is bigger than max, but it's a good practice to clear out the confusion
@Shinigami92 Shinigami92 removed the needs rebase There is a merge conflict label May 23, 2022
@Shinigami92 Shinigami92 merged commit a72f3d8 into faker-js:main May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Small issue with mersenne jsdoc
6 participants