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

feat: add isbn 10/13 fake generation support #368

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

phoenisx
Copy link

Closes #367

Description

Adds support for ISBN 10/13 strings in gofakeit library. It's inspired from https://github.com/joke2k/faker/blob/master/faker/providers/isbn

Currently added support for only en-US regions, i.e. 0 & 1 Registration Groups. We can expand this data to support other regions/countries if required in future.


Let me know your thoughts, and any fixes if necessary, coz this will be my first Golang PR 😅 🎉 🙌🏽

@brianvoe
Copy link
Owner

Sorry I missed this.

The pr looks good. The only thing i question is the categorization. Instead of creating a new one lets put it under product.

Once that update is done ill get this merged in.

Nice job!

@phoenisx
Copy link
Author

Thanks @brianvoe for the review.

I have one concern merging ISBN with Product:

  • I can either support ISBN 10 and 13 as two separate properties in Product struct, or
  • I support only one ISBN string, which by default sets to ISBN13 and if user passes an option/param (which modifies the Product() function signature) use ISBN 10 or 13 based on that option.

Which one do you think would work the best here?

@brianvoe
Copy link
Owner

Lets do the more common ISBN 13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ISBN 10 and ISBN13
2 participants