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

1693 USA driving license #2090

Merged
merged 9 commits into from
Aug 15, 2020

Conversation

sudeeptarlekar
Copy link
Contributor

Issue#

resolves #1693

Description:

Added driving licence method for USA. Default state is California, you can generate licence number for states by passing the state argument to method

@sudeeptarlekar
Copy link
Contributor Author

Can I get any reviews on this PR?

@psibi
Copy link
Member

psibi commented Jul 29, 2020

General comment on the MR: Do you think having something like this would be more nice: Faker::DrivingLicence.usa_driving_licence('Alabama') instead of passing abbreviations ?

@sudeeptarlekar
Copy link
Contributor Author

Full state names also sounds good to me, will change to use full state names

@sudeeptarlekar sudeeptarlekar force-pushed the 1693-us-driving-license branch from e0b1b1a to 8b8b3e7 Compare July 30, 2020 16:50
@sudeeptarlekar
Copy link
Contributor Author

@psibi update state codes to names

Copy link
Member

@psibi psibi left a comment

Choose a reason for hiding this comment

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

The local file changes LGTM. For the Ruby change, I would leave it to someone else from the team to review.

@psibi
Copy link
Member

psibi commented Aug 3, 2020

@Zeragamba Mind giving the ruby code a review ?

Copy link
Member

@psibi psibi left a comment

Choose a reason for hiding this comment

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

Okay, I went with reviewing the Ruby code. I think this PR looks good to be merged apart from a small comment.

@sudeeptarlekar sudeeptarlekar force-pushed the 1693-us-driving-license branch from c00a85f to 659438e Compare August 7, 2020 18:07
@sudeeptarlekar
Copy link
Contributor Author

@psibi added version and method doc, not sure why specs are failing on Ruby2.8.

@sudeeptarlekar sudeeptarlekar force-pushed the 1693-us-driving-license branch from 659438e to 0a078f4 Compare August 12, 2020 06:23
Copy link
Member

@psibi psibi left a comment

Choose a reason for hiding this comment

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

Sorry, that I missed this in the first review. One minor comment.

lib/locales/en/driving_license.yml Outdated Show resolved Hide resolved
@sudeeptarlekar sudeeptarlekar force-pushed the 1693-us-driving-license branch from 0a078f4 to 8ebc948 Compare August 14, 2020 21:09
@psibi psibi merged commit 35ec5ed into faker-ruby:master Aug 15, 2020
@psibi
Copy link
Member

psibi commented Aug 15, 2020

Thanks for your contribution!

@sudeeptarlekar sudeeptarlekar deleted the 1693-us-driving-license branch August 18, 2020 19:39
tjozwik pushed a commit to tjozwik/faker that referenced this pull request Sep 14, 2020
US driving licence for states
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.

US Driver License per state
2 participants