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

Added Antarctica in country for mobile validation #1761 #2092

Closed
wants to merge 4 commits into from

Conversation

Sambhavmahajan11
Copy link

Added Antarctica in country for mobile validation #1761
Also updated the readme

Checklist

  • [✅ ] PR contains only changes related; no stray files, etc.
  • [ ✅] README updated (where applicable)
  • Tests written (where applicable)

Added validation for Antarctica validatorjs#1761
Updated readme for Antarctica validatorjs#1761
@codecov
Copy link

codecov bot commented Oct 27, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (3116122) compared to base (531dc7f).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2092   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          104       104           
  Lines         2308      2308           
  Branches       578       578           
=========================================
  Hits          2308      2308           
Impacted Files Coverage Δ
src/lib/isMobilePhone.js 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

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

Don't forget to add tests

README.md Outdated Show resolved Hide resolved
@WikiRik
Copy link
Member

WikiRik commented Oct 28, 2022

I didn't look into the RegEx itself yesterday but it seems that the Artic bases do not really have their own phone numbers.
https://en.wikipedia.org/wiki/List_of_country_calling_codes does mention that + 672 1x is for the Australian Antartic Territory and in the table https://en.wikipedia.org/wiki/List_of_country_calling_codes#Locations_with_no_country_code it shows that the other bases are dialed based on their parent country.

Also I would not be sure how often this would be used, so I would recommend against merging this PR

@Sambhavmahajan11
Copy link
Author

This my first pr could u please approve
I will make better ones in the near future.

@pano9000
Copy link
Contributor

@Sambhavmahajan11 thank you for the PR, but I also do not see any real value in this PR, as this seems to be too much of a niche use case (if at all).
After all, from what I know, the people in the Antartic Territories are not really permanent "residents", but rather temporary inhabitants.

I would too rather see this as a "Won't Do" as well.

@profnandaa would you agree to close this issue?

@profnandaa
Copy link
Member

@pano9000 -- good by me, we can close.

@pano9000
Copy link
Contributor

@Sambhavmahajan11 again, thanks for the PR, but I need to close this PR, kindly see above for the reasons.

@pano9000 pano9000 closed this Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants