Skip to content

add(isURL): added validate_length option#1397

Merged
profnandaa merged 5 commits intovalidatorjs:masterfrom
tomgrossman:master
Aug 1, 2020
Merged

add(isURL): added validate_length option#1397
profnandaa merged 5 commits intovalidatorjs:masterfrom
tomgrossman:master

Conversation

@tomgrossman
Copy link
Copy Markdown
Contributor

fix for issue: #1396

Today the isURL function validate hard-coded for max 2083 string length. This is not part of the RFC and should be able to skip this validation.

Checklist

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

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

LGTM, thanks for your contrib! 🎉
@tux-tn -- could you have a second look on this?

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

@tomgrossman Thank you for your contribution 🎉

lib folder is generated by babel, you need to make your changes inside src/lib folder. In addition, you'll have to update existing tests (there is a test case where the URL is longer than 2083) and add new tests for your added option

@tomgrossman
Copy link
Copy Markdown
Contributor Author

@tux-tn fixed. for some reason I can't click on the "re-request review" button, it doesn't work...

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

LGTM 🎉

@profnandaa profnandaa merged commit ed86b0a into validatorjs:master Aug 1, 2020
@bruno-smaldone
Copy link
Copy Markdown

Opened related issue: #1412

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants