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

Bugfix - Wrong IP address validation #1211

Merged
merged 4 commits into from
Feb 11, 2020
Merged

Bugfix - Wrong IP address validation #1211

merged 4 commits into from
Feb 11, 2020

Conversation

molaga
Copy link
Contributor

@molaga molaga commented Dec 8, 2019

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 PR! 🎉 Please remove all changes that are unrelated to your PR -- perhaps you need to properly rebase with validator.js:master

es/lib/alpha.js Outdated Show resolved Hide resolved
es/lib/alpha.js Outdated Show resolved Hide resolved
es/lib/isMobilePhone.js Outdated Show resolved Hide resolved
@molaga
Copy link
Contributor Author

molaga commented Dec 9, 2019

Thanks for your PR! Please remove all changes that are unrelated to your PR -- perhaps you need to properly rebase with validator.js:master

@profnandaa
I've checked out from clean master, I think that previous commit to master was without running npm run build first

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.

Still we have unwanted files, alpha.js, isMobilePhone.js, etc. Unstage them, see the guide here

-na

@profnandaa profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label Dec 30, 2019
barintsights and others added 2 commits January 24, 2020 15:50
# Conflicts:
#	es/lib/isMobilePhone.js
#	lib/isMobilePhone.js
#	validator.js
#	validator.min.js
Copy link
Contributor Author

@molaga molaga left a comment

Choose a reason for hiding this comment

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

@profnandaa I've excluded the irrelevant code

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 the update. LGTM 👍

@profnandaa
Copy link
Member

@molaga -- ooh, one last thing before we land this; can you please add a few test cases, mostly invalid including what you pointed out in #1209

@molaga
Copy link
Contributor Author

molaga commented Feb 11, 2020

@molaga -- ooh, one last thing before we land this; can you please add a few test cases, mostly invalid including what you pointed out in #1209

👍

@profnandaa profnandaa merged commit c109ace into validatorjs:master Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 needs-update For PRs that need to be updated before landing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants