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): en-GB enhance #1971

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ihmpavel
Copy link

Update mobile phone validation for en-GB

  • The first digit after 07 should not be zero
  • Updated tests

Checklist

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

Inspired by #1164

Also I am wondering, whether support more phone numbers in the UK.

Quorra link with explanation https://qr.ae/pvAeI7

@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (83d6ffd) to head (8f6c21b).
Report is 2 commits behind head on master.

❗ Current head 8f6c21b differs from pull request most recent head 967a039. Consider uploading reports for the commit 967a039 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            master     #1971    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          108       103     -5     
  Lines         2482      2097   -385     
  Branches       627       473   -154     
==========================================
- Hits          2482      2097   -385     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ihmpavel ihmpavel changed the title feat: isMobilePhone en-GB enhance feat(isMobilePhone): en-GB enhance May 18, 2022
@@ -34,7 +34,7 @@ const phones = {
'el-GR': /^(\+?30|0)?(69\d{8})$/,
'en-AU': /^(\+?61|0)4\d{8}$/,
'en-BM': /^(\+?1)?441(((3|7)\d{6}$)|(5[0-3][0-9]\d{4}$)|(59\d{5}))/,
'en-GB': /^(\+?44|0)7\d{9}$/,
'en-GB': /^(\+?44|0)7[1-9]\d{8}$/,
Copy link
Member

Choose a reason for hiding this comment

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

@tux-tn and @profnandaa, do we need need to allow pagers and personal numbering as part of the phone numbers?
Screenshot 2022-06-03 at 04 57 32

Copy link
Member

Choose a reason for hiding this comment

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

what is personal numbering? Are people still using pagers these days?
image

@profnandaa profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label Jul 23, 2022
@profnandaa profnandaa added the mc-to-land Just merge-conflict standing between the PR and landing. label Jun 26, 2023
@profnandaa
Copy link
Member

@ihmpavel - possible to fix the merge conflict here? Should be okay to land after that.

profnandaa
profnandaa previously approved these changes Jun 26, 2023
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.

just the merge conflicts and should be good.

@rubiin rubiin removed the 🧹 needs-update For PRs that need to be updated before landing label May 10, 2024
@rubiin
Copy link
Member

rubiin commented May 10, 2024

@ihmpavel can you fix the merge conflicts

@rubiin rubiin added mc-to-land Just merge-conflict standing between the PR and landing. ✅ LGTM and removed mc-to-land Just merge-conflict standing between the PR and landing. labels May 10, 2024
@rubiin rubiin requested a review from profnandaa May 10, 2024 11:26
@ihmpavel
Copy link
Author

@rubiin Conflicts resolved

@ihmpavel
Copy link
Author

It looks like mc-to-land label is redundant

@rubiin rubiin removed the mc-to-land Just merge-conflict standing between the PR and landing. label May 30, 2024
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.

None yet

4 participants