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

Fixes #73, removes validator.check from README.md and adds valid form… #74

Merged
merged 2 commits into from
Jun 5, 2015

Conversation

mmerkes
Copy link
Contributor

@mmerkes mmerkes commented Jun 2, 2015

…at checker

@madarche
Copy link
Collaborator

madarche commented Jun 4, 2015

@mmerkes thanks for this PR that will nicely improve the doc!

Some remarks:

  • Your PR should also remove the line var check = require('validator').check;
  • Your PR should better use the addFormat method as shown in the format-tests.js file

@mmerkes
Copy link
Contributor Author

mmerkes commented Jun 4, 2015

Thanks fro the feedback! I will update.

@mmerkes
Copy link
Contributor Author

mmerkes commented Jun 5, 2015

@madarche I tidied up the example and added another example using convict.addFormat() per your suggestion.

@madarche
Copy link
Collaborator

madarche commented Jun 5, 2015

OK, it's very convincing :-) Thanks.

@zaach could you merge this PR please?

@zaach
Copy link
Contributor

zaach commented Jun 5, 2015

👍

zaach added a commit that referenced this pull request Jun 5, 2015
Fixes #73, removes validator.check from README.md and adds valid form…
@zaach zaach merged commit a269cd2 into mozilla:master Jun 5, 2015
@mmerkes
Copy link
Contributor Author

mmerkes commented Jun 5, 2015

Cool. Thanks for review, discussion and merge! I can very persuasive ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants