Skip to content

Conversation

@neilime
Copy link
Contributor

@neilime neilime commented Feb 5, 2021

At the moment, isIPRange function only supports IP v4, this PR allows to choose v4 or v6 or both (like isIP function)

Checklist

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

@codecov
Copy link

codecov bot commented Feb 5, 2021

Codecov Report

Merging #1594 (f7562ab) into master (deb1d1e) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1594   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          100       100           
  Lines         1807      1820   +13     
=========================================
+ Hits          1807      1820   +13     
Impacted Files Coverage Δ
src/lib/isIPRange.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update deb1d1e...f7562ab. Read the comment docs.

@fredleger
Copy link

Nice one ! Need this too

@renanmontebelo
Copy link
Contributor

Would this be a duplicate of #1568? Yours looks like it's a more complete solution though.
Maybe a discussion should be setup and close one of the PRs in favor of the other.

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

Sorry for the late review. Thank you @neilime for your contribution! I really like your approach and the fact that you used the same logic behind isIP.
Can you please remove validator.js and validator.min.js (We stopped tracking those files in the library) and discard the unrelated changes in src/lib/isMobilePhone ?
Thank you 🎉

@tux-tn tux-tn added needs-more-review 🧹 needs-update For PRs that need to be updated before landing labels Mar 8, 2021
@neilime
Copy link
Contributor Author

neilime commented Mar 9, 2021

@tux-tn I have rebased with master, let me know if everything looks good to you

@neilime neilime requested a review from tux-tn March 9, 2021 17:38
@tux-tn
Copy link
Member

tux-tn commented Mar 9, 2021

@neilime LGTM ! thank you for making the necessary changes 🎉

@tux-tn tux-tn added ready-to-land For PRs that are reviewed and ready to be landed and removed 🧹 needs-update For PRs that need to be updated before landing needs-more-review labels Mar 9, 2021
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 contrib! 🎉

@profnandaa profnandaa merged commit 5d6db63 into validatorjs:master Apr 17, 2021
@neilime neilime deleted the feat/is-ip-range-version-4-6 branch April 17, 2021 15:30
@tux-tn tux-tn mentioned this pull request Oct 2, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land For PRs that are reviewed and ready to be landed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants