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
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
Copy link
Contributor Author

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

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
@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