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

Be able to validate Denmark's mobile numbers #142

Merged
merged 1 commit into from
May 2, 2024

Conversation

waterkip
Copy link
Contributor

Our app has an issue with Danish numbers, which cannot be validated as either fixed line or mobile. This is a minor inconvenience to our users.

The problem is that upstream has fixed lines and mobiles lines defined as the same regexp and Number::Phone strips the checks for is_mobile and is_fixed when these are the same, as found in build-data.stubs and build-tests.pl, introduced by 02f12d4.

I've created an upstream PR to improve the Danish number checks so we in Perl are going to be aware what Denmark's mobile and landline number are. I've ran build-data.sh --force with the data from the PR at Google and the tests in t/stubs.t work as intended.

Only merge after google/libphonenumber#3417 has been merged into the Google code base.

@DrHyde
Copy link
Owner

DrHyde commented Mar 26, 2024

Thanks, I'll keep an eye on this - Google are tracking it as https://issuetracker.google.com/issues/331311053

Our app has an issue with Danish numbers, which cannot be validated as
either fixed line or mobile. This is a minor inconvenience to our users.

The problem is that upstream has fixed lines and mobiles lines defined
as the same regexp and Number::Phone strips the checks for is_mobile and
is_fixed when these are the same, as found in build-data.stubs and
build-tests.pl, introduced by 02f12d4.

I've created an upstream PR to improve the Danish number checks so we in
Perl are going to be aware what Denmark's mobile and landline number
are. I've ran build-data.sh --force with the data from the PR at Google and
the tests in t/stubs.t work as intended.

Only merge after google/libphonenumber#3417 has
been merged into the Google code base. Google fixed it a weebit
different from how I did it in commit
f64bace16b9319c5eedfaa5723c1b60d72cc5311.

Signed-off-by: Wesley Schwengle <waterkip@cpan.org>
@waterkip
Copy link
Contributor Author

waterkip commented Apr 2, 2024

Google has made updates available, which includes metadata changes for Denmark: google/libphonenumber@f64bace

I've ran ./build-data.sh and reran t/stubs.t, which needed a small update to reflect the changes Google made. Those changes are now found in the PR.

@DrHyde
Copy link
Owner

DrHyde commented May 2, 2024

Thanks, looks good. I'll include this in the next release.

@DrHyde DrHyde merged commit cb68774 into DrHyde:master May 2, 2024
@waterkip waterkip deleted the denmark-distinction-ready branch May 2, 2024 20:44
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.

None yet

2 participants