Fix Phone number long number and other updates#2842
Merged
stefannibrasil merged 5 commits intofaker-ruby:mainfrom Nov 13, 2023
Merged
Fix Phone number long number and other updates#2842stefannibrasil merged 5 commits intofaker-ruby:mainfrom
stefannibrasil merged 5 commits intofaker-ruby:mainfrom
Conversation
- moves phones country code to `phone_number.country_code` to avoid mixing it with address country code; - removes extensions from the phone number generation. Extensions added more numbers to the cell phone; - update `en.phone_number.country_code` locale to include only a sample of random country codes, and removes extra digits such as area code from them; - fixes Phone number area and exchange code with no locales: if there is no locale set, falls back to the `en.phone_number` area and exchange code generators; - updates YARD docs about the generators, and add more tests; - updates documentation;
stefannibrasil
commented
Oct 21, 2023
lib/locales/de-CH.yml
Outdated
| - viral | ||
| - vlog | ||
| phone_number: | ||
| country_code: ["41"] |
Contributor
Author
There was a problem hiding this comment.
Most of the locale changes are like this one. country_code was not inside of phone_number. It was confusing, so I fixed them to all be together.
stefannibrasil
commented
Oct 21, 2023
| assert_match(ca_number_validation_regex, Faker::PhoneNumber.phone_number) | ||
| end | ||
|
|
||
| def test_en_ca_phone_methods_return_nil_for_nil_locale |
Contributor
Author
There was a problem hiding this comment.
I believe this was implemented before we had the locales. I changed this to generate us/ca area and exchange code values if there is no locale set. It's better than returning nil if there is no locale set, IMO.
stefannibrasil
commented
Oct 21, 2023
| assert_kind_of String, Faker::Address.village | ||
| assert_equal 'գ.', Faker::Address.village_prefix | ||
| assert_includes %w[1-374 374], Faker::Address.country_code | ||
| assert_equal '+374', Faker::PhoneNumber.country_code |
Contributor
Author
There was a problem hiding this comment.
374 is Armenia's country code.
Co-authored-by: Raymond Sapida <[email protected]>
Co-authored-by: Raymond Sapida <[email protected]>
thdaraujo
approved these changes
Nov 13, 2023
Contributor
thdaraujo
left a comment
There was a problem hiding this comment.
LGTM, thank you @stefannibrasil and @raysapida ! 🙌
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2047
Summary of the changes
phone_number.country_codeto avoid mixing it with address country code;en.phone_number.country_codelocale to include only a sample of random country codes, and removes extra digits such as area code from them;en.phone_numberarea and exchange code generators;Review notes
When I save a YAML file, my editor formats it. So the file changes are mainly because of that. Sorry for the number of files changed!
The documentation is helpful to guide your review as well. Thanks!