Skip to content

Consider option "allowEmptyString" in examples #13358

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

Merged
merged 1 commit into from
Mar 17, 2020

Conversation

W0rma
Copy link
Contributor

@W0rma W0rma commented Mar 16, 2020

According to https://symfony.com/doc/4.4/reference/constraints/Length.html#allowemptystring the option "allowEmptyString" is mandatory if "min" is set (only for symfony 4.4). Therefore I'd like to consider this in the examples.

@W0rma W0rma requested a review from xabbuh as a code owner March 16, 2020 10:16
@W0rma W0rma changed the base branch from master to 4.4 March 16, 2020 10:17
@@ -42,7 +42,8 @@ and "50", you might add the following:
* min = 2,
* max = 50,
* minMessage = "Your first name must be at least {{ limit }} characters long",
* maxMessage = "Your first name cannot be longer than {{ limit }} characters"
* maxMessage = "Your first name cannot be longer than {{ limit }} characters",
* allowEmptyString = true
Copy link
Member

Choose a reason for hiding this comment

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

I would set this option to false. Allowing the empty string in this context doesn't seem to make much sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I set this option to false

@W0rma W0rma force-pushed the fix-examples-length-constraint branch from 870b989 to a0ffdb4 Compare March 16, 2020 19:16
@javiereguiluz
Copy link
Member

Thank you @W0rma.

@javiereguiluz javiereguiluz merged commit 26228a1 into symfony:4.4 Mar 17, 2020
@W0rma W0rma deleted the fix-examples-length-constraint branch October 1, 2024 05:39
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