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

Add Faker::PhoneNumber.country_code #1426

Merged
merged 11 commits into from
Oct 22, 2018

Conversation

AmrAdelKhalil
Copy link
Contributor

Fixes #558

Copy link
Member

@vbrazo vbrazo left a comment

Choose a reason for hiding this comment

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

Overall looks good 👍

Couple of things:

  • Could you add documentation for the new methods? Take a look at doc/phone_number.md
  • Could you add tests for these methods as well?

Here is another PR that tried to implement something similar but we couldn't finish the changes to merge them #523

@vbrazo vbrazo changed the title Add country code Add Faker::Address.country_code Oct 20, 2018
@vbrazo vbrazo changed the title Add Faker::Address.country_code Add Faker::PhoneNumber.country_code Oct 20, 2018
@AmrAdelKhalil
Copy link
Contributor Author

@vbrazo Hey, I added required changes as you said.

@vbrazo vbrazo merged commit 784c987 into faker-ruby:master Oct 22, 2018
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
* adds country code for phone numbers and cel phones

* use interpolation for string, move country codes to phone_number file, Add and update country codes data

* update documentation with new methods

* add + sign in front of country code

* update data of country codes, replace spaces with dashes, sort  strings of country codes, add new line for text

* add test cases for new country code related functions

* update test cases, refactor code

* Update CHANGELOG.md

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

Successfully merging this pull request may close these issues.

3 participants