-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
phone validation for malawi ('en-MW') #1823
base: master
Are you sure you want to change the base?
Conversation
This reverts commit 13651ea. reverted
@tux-tn another one, is this right? |
Codecov Report
@@ Coverage Diff @@
## master #1823 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 102 102
Lines 2029 2029
Branches 457 457
=========================================
Hits 2029 2029
Continue to review full report at Codecov.
|
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.
Looks good
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.
Please add tests too
@tux-tn done! Tests added and passed. |
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.
Hi can we add this?
@rubiin have added the test cases |
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.
@rohitnairtech Thank you for PR! Sorry for the review delay 😅
Can you please address my 2 comments below?
if (!isIP(host) && !isFQDN(host, options) && (!ipv6 || !isIP(ipv6, 6))) { | ||
return false; | ||
} | ||
|
||
host = host || ipv6; | ||
|
||
if (options.host_whitelist && !checkHost(host, options.host_whitelist)) { |
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.
Can you please revert those unrelated changes?
@@ -117,6 +117,7 @@ const phones = { | |||
'vi-VN': /^((\+?84)|0)((3([2-9]))|(5([25689]))|(7([0|6-9]))|(8([1-9]))|(9([0-9])))([0-9]{7})$/, | |||
'zh-CN': /^((\+|00)86)?1([3456789][0-9]|4[579]|6[2567]|7[01235678]|9[012356789])[0-9]{8}$/, | |||
'zh-TW': /^(\+?886\-?|0)?9\d{8}$/, | |||
'en-MW': /^(\+?265|0)(\d{7}|\d{9})$/, |
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.
According to ITU Document and Wikipedia there is some carrier prefixes for mobile phones in both old and new malawi number plan. Can you take those prefixes into consideration please?
Checklist