Skip to content

Add support for IP version 4 or 6 for isIPRange function#1594

Merged
profnandaa merged 1 commit intovalidatorjs:masterfrom
hoverkraft-tech:feat/is-ip-range-version-4-6
Apr 17, 2021
Merged

Add support for IP version 4 or 6 for isIPRange function#1594
profnandaa merged 1 commit intovalidatorjs:masterfrom
hoverkraft-tech:feat/is-ip-range-version-4-6

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 •
edited
Loading

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

fredleger commented Feb 5, 2021

Nice one ! Need this too

@renanmontebelo
Copy link
Contributor

renanmontebelo commented Feb 6, 2021

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.

tux-tn
tux-tn requested changes Mar 8, 2021
View reviewed changes
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
profnandaa
profnandaa approved these changes Apr 17, 2021
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 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
@neilime neilime mentioned this pull request Apr 17, 2021
Closed
@profnandaa profnandaa mentioned this pull request Apr 19, 2021
Closed
@tux-tn tux-tn mentioned this pull request Oct 2, 2021
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@profnandaa profnandaa profnandaa approved these changes

@tux-tn tux-tn tux-tn approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@neilime @fredleger @renanmontebelo @tux-tn @profnandaa