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

DOCS: Avoid passing incorrect/unneeded args to faker.* methods #707

Merged
merged 1 commit into from
Jul 11, 2019
Merged

DOCS: Avoid passing incorrect/unneeded args to faker.* methods #707

merged 1 commit into from
Jul 11, 2019

Conversation

bantic
Copy link
Contributor

@bantic bantic commented Jul 10, 2019

faker.company.companyName for its first arg expects either a format string
or an integer in the range [0,2]:
https://github.com/Marak/faker.js/blob/3a4bb358614c1e1f5d73f4df45c13a1a7aa013d7/lib/company.js#L26-L39

Passing a value greater than 2 results in a returned value of string parameter is required! instead of a faked name. This wasn't obvious because
the number of generated rows is dynamic (via getRandomInt), so it only
shows up if the # of generated rows is enough to trigger the error, and then
you have to scroll to the bottom of the table to notice it.

Also remove the argument passed to department because it doesn't accept an argument:
https://github.com/Marak/faker.js/blob/3a4bb358614c1e1f5d73f4df45c13a1a7aa013d7/lib/commerce.js#L22-L24

Ditto for productName:
https://github.com/Marak/faker.js/blob/3a4bb358614c1e1f5d73f4df45c13a1a7aa013d7/lib/commerce.js#L31-L35

The issue can be seen at "sorting" page of the current published docs site. If you reload the page enough times so that the table renders more than 3 rows, the 4th row will say "string parameter is required!":
image

faker.company.companyName for its first arg expects either a format string
or an integer in the range [0,2]:
https://github.com/Marak/faker.js/blob/3a4bb358614c1e1f5d73f4df45c13a1a7aa013d7/lib/company.js#L26-L39

Passing a value greater than 2 results in a returned value of `string
parameter is required!` instead of a faked name. This wasn't obvious because
the number of generated rows is dynamic (via `getRandomInt`), so it only
shows up if the # of generated rows is enough to trigger the error, and then
you have to scroll to the bottom of the table to notice it.

Also remove the argument passed to `department` because it doesn't accept an argument:
https://github.com/Marak/faker.js/blob/3a4bb358614c1e1f5d73f4df45c13a1a7aa013d7/lib/commerce.js#L22-L24

Ditto for `productName`:
https://github.com/Marak/faker.js/blob/3a4bb358614c1e1f5d73f4df45c13a1a7aa013d7/lib/commerce.js#L31-L35
@bantic bantic added the docs label Jul 10, 2019
@bantic bantic added this to the Prepare for 2.0 release milestone Jul 10, 2019
@bantic bantic requested a review from cyril-sf July 10, 2019 19:18
Copy link
Member

@mixonic mixonic left a comment

Choose a reason for hiding this comment

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

Yipe, good digging.

@bantic bantic merged commit c06324c into Addepar:master Jul 11, 2019
@bantic bantic deleted the bantic/fix-faker-usage branch July 11, 2019 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants