Skip to content

Conversation

@houmark
Copy link
Contributor

@houmark houmark commented Apr 30, 2016

Very simple, but makes the directive useful for me.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.154% when pulling 65dffc9 on houmark:us-phone-validation-fix into 9d41763 on assisrafael:master.

@assisrafael
Copy link
Owner

Thank you for the pull request.
Can you implement a test case to cover this bug?

@houmark
Copy link
Contributor Author

houmark commented May 10, 2016

This one really is a one line no-brainer, but without this pulled the phone number cannot validate for me at all (when the phone number is just a number) which makes the entire module useless. I wonder how it stayed in so long? ;)

If you insist I will go it a go, but it will be some time before I can find time to do it.

@assisrafael
Copy link
Owner

Only a test case can prevent this feature to be accidentally removed in the future and break your code.

Model value is added as a number which ensures it can validate (which
is what this PR is for). I noticed other tests are adding the phone
number as a string (in br-phone.test.js), which probably defeats the
test.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 97.297% when pulling 68d83bd on houmark:us-phone-validation-fix into 9d41763 on assisrafael:master.

@houmark
Copy link
Contributor Author

houmark commented Aug 27, 2016

@assisrafael I finally had time to get back around to do the test you asked for. Hoping to see this merged soon.

@assisrafael assisrafael merged commit 2ea6056 into assisrafael:master Aug 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants