This repository has been archived by the owner on Jan 30, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 49
Conversation
This file contains 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
malukenho
approved these changes
Aug 31, 2018
All unit tests are successful - with and without these changes?! |
Ocramius
suggested changes
Aug 31, 2018
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.
Requires addition of example phone numbers in the test suite
Brazilian mobile numbers begin with an 9 before the area code.
michalbundyra
force-pushed
the
patch-1
branch
from
September 22, 2019 19:53
8286d72
to
2b54cef
Compare
All brazilian mobile numbers have digit 9 after area code (two first digits) then number 6-9 and another 7 digits.
@rwrz Would you be able to review my changes? I've updated area code for fixed and mobile phones and added 9 after area code for mobiles. |
@webimpress all good man, tested with a few real numbers... working 100%. |
michalbundyra
added a commit
that referenced
this pull request
Sep 25, 2019
Fix Brazil PhoneNumber Regexp # Conflicts: # src/Validator/PhoneNumber/BR.php
michalbundyra
added a commit
that referenced
this pull request
Sep 25, 2019
Thanks, @rwrz! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
For some reason, all REGEX was using
\\d
instead of\d
... this should fix the problem.And Brazilian numbers changed mobile numbers to begin with an
9
before the area code... fixed this regex too.Are you fixing a bug?
Detail how the bug is invoked currently.
Currently, any BR number should work, because of a typo on REGEX.
\\d
instead of\d
.Someone should verify all others files, since it is a typo...
Detail the original, incorrect behavior.
There is a problem with BR mobile numbers too. Now, all brazilian mobile numbers starts with a
9
.Detail the new, expected behavior.
Fixed to accept the 9.
Add a
CHANGELOG.md
entry for the fix.Sorry, can someone do that? I'm changing here in Github...
Provide a test case
Sorry, can someone do that? I'm changing here in Github...