Changes to avoid issues with normalizing a phone number started with reserved +0 country code. #180

Merged
merged 2 commits into from Sep 13, 2014

Conversation

Projects
None yet
2 participants
@rrott
Contributor

rrott commented Sep 12, 2014

We've faced an issue with normalizing a test phone number that starts with reserved +0 country code. The issue was with normalize() method that returned '0' as a normalized phone number when we used "+00 000 000 000" number in our test cases. It's not a big dial to use another test number but from my point of view such wrong numbers should have not be recognized as a valid # as +0 zone is not assigned to a country.
Based on the fact that E.164 does not use that zone and there is no plans to include it to this standard I suggest to get rid of it and that's why I am sending this pull request.
But just in case I've only commented this part out to be able to return it back in the future if they assign this code to a country(but I believe it will not be a the nearest future because of bureaucracy) And anyway it will be better to add this zone and an appropriate number format for it only when and if it happens.

@rrott

This comment has been minimized.

Show comment
Hide comment
@rrott

rrott Sep 12, 2014

Contributor

Sorry for splitting 2 pull requests into one. Did not know that pushing to an existing forked repo will automatically add such updates to an existing pull request =(

Contributor

rrott commented Sep 12, 2014

Sorry for splitting 2 pull requests into one. Did not know that pushing to an existing forked repo will automatically add such updates to an existing pull request =(

@floere

This comment has been minimized.

Show comment
Hide comment
@floere

floere Sep 13, 2014

Owner

No problem. To avoid having commits being automatically added, use separate branches for each pull request.

I am not sure I understand the test case – why do you have a test case for a 0 number? I'm curious as to what the goal of it is. The E164 standard is quite silent on the topic of reserved country codes (iirc).

I'll pull this pull request and will probably add a handler for reserved numbers which returns false for the plausibility of reserved numbers.

Many thanks for your work!

Owner

floere commented Sep 13, 2014

No problem. To avoid having commits being automatically added, use separate branches for each pull request.

I am not sure I understand the test case – why do you have a test case for a 0 number? I'm curious as to what the goal of it is. The E164 standard is quite silent on the topic of reserved country codes (iirc).

I'll pull this pull request and will probably add a handler for reserved numbers which returns false for the plausibility of reserved numbers.

Many thanks for your work!

floere added a commit that referenced this pull request Sep 13, 2014

Merge pull request #180 from rrott/master
Changes to avoid issues with normalizing a phone number started with reserved +0 country code.

@floere floere merged commit 5d824cc into floere:master Sep 13, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@floere

This comment has been minimized.

Show comment
Hide comment
@floere

floere Sep 13, 2014

Owner

Included in 2.3.0.

Owner

floere commented Sep 13, 2014

Included in 2.3.0.

@rrott

This comment has been minimized.

Show comment
Hide comment
@rrott

rrott Sep 14, 2014

Contributor

Thank you!
It was just a random test case that catched the bug.

Contributor

rrott commented Sep 14, 2014

Thank you!
It was just a random test case that catched the bug.

@floere

This comment has been minimized.

Show comment
Hide comment
@floere

floere Sep 14, 2014

Owner

Cheers! :)

Owner

floere commented Sep 14, 2014

Cheers! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment