Skip to content

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

Merged
profnandaa merged 7 commits intovalidatorjs:masterfrom
stinkymonkeyph:master
Jul 29, 2020
Merged

feat(isMobilePhone): add support for Philippine mobile no#1388
profnandaa merged 7 commits intovalidatorjs:masterfrom
stinkymonkeyph:master

Conversation

@stinkymonkeyph
Copy link
Copy Markdown
Contributor

Add support for Philippine mobile no.

Add support for Philippine mobile no.
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

LGTM 🎉

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

Copy link
Copy Markdown
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