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

[RFR] Docs: Move built-in validators above custom validation #3363

Merged
merged 3 commits into from
Sep 17, 2019

Conversation

heyfife
Copy link
Contributor

@heyfife heyfife commented Jun 25, 2019

I put the easy way to do validation first followed by advanced, custom validation methods.

We had custom required() validators all over our code since that's the first way validation is described in the docs. Hopefully this reordering will help others avoid the extra, custom work when built-in functionality already exists to cover common cases.

Put the easy way first followed by advanced, custom validation methods.
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

Sorry, the documentation doesn't make sense the way you ordered it, as per Input validation is introduced after the list of existing validators. Besides, the introduction of the "Validation" part still mentions global validation before per field validation.

@fzaninotto
Copy link
Member

Bump, can you update your PR?

@heyfife
Copy link
Contributor Author

heyfife commented Jul 22, 2019

Yes. Will do this week.

Re-ordered to make the flow more readable. Introduce field validation with the built-in validators first then discuss custom validators.
@fzaninotto
Copy link
Member

Again, this doesn't make sense, and the repetition of the validators makes the doc harder to read.

Removed duplicate field validation section
@heyfife
Copy link
Contributor Author

heyfife commented Aug 22, 2019

I agree, it did not make sense. I left in the built-in validation section at the top and meant to remove it. It should be clear now.

@fzaninotto fzaninotto merged commit 2bd76ee into marmelab:master Sep 17, 2019
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto added this to the 2.9.7 milestone Sep 17, 2019
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.

2 participants