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

feat(isMobilePhone): add support for Philippine mobile no #1388

Merged
merged 7 commits into from
Jul 29, 2020

Conversation

stinkymonkeyph
Copy link
Contributor

Add support for Philippine mobile no.

Add support for Philippine mobile no.
Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check Contributing guidelines and add the missing parts:

  • Adding your code to src/lib and not lib
  • Writing test cases for your new validation
  • Adding an entry in README file for your new locale

@stinkymonkeyph
Copy link
Contributor Author

Hi @tux-tn done with the changes but it won't allow me to re-request review

Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stinkymonkeyph thank you for making the necessary changes, you don't need to create a new pull request. I see that your regex is validating numbers starting with +63 followed by 10 decimals, isn't that the international format of philippine mobile numbers? Can you add the format for domestic callers as well?

@stinkymonkeyph
Copy link
Contributor Author

@stinkymonkeyph thank you for making the necessary changes, you don't need to create a new pull request. I see that your regex is validating numbers starting with +63 followed by 10 decimals, isn't that the international format of philippine mobile numbers? Can you add the format for domestic callers as well?

Yeah sure, I'll add them as well.

@profnandaa profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label Jul 28, 2020
@stinkymonkeyph
Copy link
Contributor Author

Hi sorry for the late update, been busy with several things. I'm using this package in one of our projects and it was missing ph support. Thank guys for all the good work.

Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@stinkymonkeyph
Copy link
Contributor Author

LGTM 🎉

Great, looking forward to seeing it on next build :)

Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too, thanks for your contrib! 🎉

@profnandaa profnandaa merged commit af36196 into validatorjs:master Jul 29, 2020
@profnandaa profnandaa mentioned this pull request Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 needs-update For PRs that need to be updated before landing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants