Skip to content

Add IP Address Validation and Separate IP and URI Logic to Files#619

Closed
DavidTPate wants to merge 1 commit intohapijs:6.1.0from
DavidTPate:separate-ip-and-uri
Closed

Add IP Address Validation and Separate IP and URI Logic to Files#619
DavidTPate wants to merge 1 commit intohapijs:6.1.0from
DavidTPate:separate-ip-and-uri

Conversation

@DavidTPate
Copy link
Contributor

This PR adds the ability to do IP address validation and updates the handling of the components used to build Regular Expressions for both IP and URI validation into files. Additionally, this replaces the old usage of Array.reduce() which was occurring before in both IP and URI validation construction in favor of the standard for(...) loop.

This resolves #607 and #617

@DavidTPate
Copy link
Contributor Author

@Marsup I got them split out where I thought they should be. I left the validation of options within String still since that is where I think it belongs, I can move it if desired to the appropriate file.

lib/language.js Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Regroup those under one option.

@Marsup Marsup added the feature New functionality or improvement label Mar 27, 2015
@Marsup Marsup added this to the 6.1.0 milestone Mar 27, 2015
@Marsup Marsup self-assigned this Mar 27, 2015
lib/string.js Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need that outside the loop.

@Marsup
Copy link
Collaborator

Marsup commented Mar 27, 2015

You're missing many line breaks in the tests as well but I won't comment on all of them.

@DavidTPate
Copy link
Contributor Author

Gotcha. I'll go through on the pieces you commented on, as well as the rest of the code. I've been getting more familiar with the coding style from the contrib repo.

@DavidTPate
Copy link
Contributor Author

@Marsup I think it's all set now. I went through and tried to find all the syntax issues I could and addressed your feedback. I also figured out what I did wrong when trying to squash things before (I wasn't force pushing before), so I squashed the changes together.

lib/string.js Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

simply options

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

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants