8000 Added a note about the usage of NotBlank by javiereguiluz · Pull Request #10407 · symfony/symfony-docs · GitHub
[go: up one dir, main page]

Skip to content

Added a note about the usage of NotBlank #10407

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

Closed
wants to merge 7 commits into from

Conversation

javiereguiluz
Copy link
Member
@javiereguiluz javiereguiluz commented Sep 26, 2018

Fixes #4244 and fixes #10357.

Copy link
@ping-localhost ping-localhost left a comment

Choose a reason for hiding this comment

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

I have a tiny comment about wording, but other than that, LGTM! 👍

@@ -88,6 +88,14 @@ Basic Usage
}
}

.. note::

As it happens with the rest of constraints, ``null`` values and empty strings

Choose a reason for hiding this comment

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

How about As with the rest of the constraints?

javiereguiluz added a commit that referenced this pull request Sep 28, 2018
This PR was squashed before being merged into the 2.8 branch (closes #10407).

Discussion
----------

Added a note about the usage of NotBlank

Fixes #4244 and fixes #10357.

Commits
-------

24ed17e Added a note about the usage of NotBlank
are considered valid values. Otherwise, in addition to validating this value,
you would also be requiring it, making it impossible to be optional. That's
why it's common to combine this constraint with
:doc:`NotBlank </reference/constraints/NotBlank>`.
Copy link
Member

Choose a reason for hiding this comment

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

I thought if we should maybe reword this a bit:

As with most of the other constraints, null values and empty strings are considered valid values. This is to allow them to be optional values. If the values is mandatory, a common solution is to combine this constraint with NotBlank.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that your wording is better. Could you please create a pull request for it? Thanks a lot!

Copy link
Member

Choose a reason for hiding this comment

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

here we go: #10418

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
0