-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Fix Phone number long number and other updates #2842
Fix Phone number long number and other updates #2842
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;
lib/locales/de-CH.yml
Outdated
@@ -3870,8 +3868,9 @@ de-CH: | |||
- viral | |||
- vlog | |||
phone_number: | |||
country_code: ["41"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -54,11 +56,4 @@ def test_validity_of_phone_method_output | |||
|
|||
assert_match(ca_number_validation_regex, Faker::PhoneNumber.phone_number) | |||
end | |||
|
|||
def test_en_ca_phone_methods_return_nil_for_nil_locale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -22,7 +22,7 @@ def test_hy_address_methods | |||
assert_equal 'ք.', Faker::Address.city_prefix | |||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
374
is Armenia's country code.
Co-authored-by: Raymond Sapida <[email protected]>
Co-authored-by: Raymond Sapida <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @stefannibrasil and @raysapida ! 🙌
Fixes #2047
Summary of the changes
phone_number.country_code
to avoid mixing it with address country code;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;en.phone_number
area 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!