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.

Sign up for GitHub

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

Jump to bottom

feat(isMobilePhone): added Qatar and dev numbers #2239

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

feat(isMobilePhone): added Qatar and dev numbers #2239

wants to merge 16 commits into from

Conversation

kishoreknowapp
Copy link

Checklist

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

@codecov
Copy link

codecov bot commented Jun 24, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4f63909) 99.95% compared to head (bcbd56a) 99.95%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #2239   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files         107      107           
  Lines        2414     2414           
  Branches      607      607           
=======================================
  Hits         2413     2413           
  Partials        1        1
Impacted Files Coverage Δ
src/lib/isMobilePhone.js 100.00% <ø> (ø)

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

@rubiin rubiin requested review from pano9000, profnandaa and WikiRik and removed request for pano9000 and profnandaa June 25, 2023 19:26
@WikiRik
Copy link
Member

WikiRik commented Jun 25, 2023

Thanks for opening this PR. Please take a look at previously merged PRs like #2209 to get an idea on what we expect in order to review this

profnandaa
profnandaa requested changes Jul 24, 2023
View reviewed changes
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.

Thanks for your contribution. Pls address the comments.

lib/isOctal.js
@@ -0,0 +1,15 @@
"use strict";
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove the stray file.

test/isMobilePhone.js
@@ -0,0 +1,11 @@
import assert from 'assert';
Copy link
Member

Choose a reason for hiding this comment

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

nit: add the appropriate tests for the new regex, can see previous PRs as @WikiRik had suggested.

@profnandaa profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@profnandaa profnandaa profnandaa requested changes

@WikiRik WikiRik Awaiting requested review from WikiRik

@pano9000 pano9000 Awaiting requested review from pano9000

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
🧹 needs-update For PRs that need to be updated before landing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kishoreknowapp @WikiRik @profnandaa