Skip to content

Conversation

@Morphyish
Copy link
Contributor

This PR allow users to add custom generators when using the faker library.

The syntax is similar to the current generators for casual.

Related open issue: #97

Copy link
Owner

@ardeois ardeois left a comment

Choose a reason for hiding this comment

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

Very nice, thank you for your contribution!
I have 2 little comments before I would merge, if you could check them out.

Thanks!

src/index.ts Outdated
return getCasualCustomScalarValue(customScalar, opts);
}

return getFakerCustomScalarValue(customScalar, opts);
Copy link
Owner

Choose a reason for hiding this comment

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

You should probably check if the value is faker and throw an error if the value is unknown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had one, but typescript told me the value couldn't be unknown. I'll add it back!

@Morphyish Morphyish requested a review from ardeois November 15, 2022 17:34
Copy link
Owner

@ardeois ardeois left a comment

Choose a reason for hiding this comment

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

LGTM thanks for the udpates!

@ardeois ardeois self-assigned this Nov 16, 2022
@ardeois ardeois added the minor Increment the minor version when merged label Nov 16, 2022
@ardeois
Copy link
Owner

ardeois commented Nov 16, 2022

@Morphyish could you fix the lint so I can merge please

@Morphyish
Copy link
Contributor Author

@ardeois Done, linter should be good now.

@ardeois ardeois enabled auto-merge (squash) November 17, 2022 16:46
@ardeois ardeois merged commit 6d91d49 into ardeois:main Nov 17, 2022
@Morphyish Morphyish deleted the feat-add-faker-custom-generators branch November 18, 2022 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants