Skip to content

Add Canonicalize option to Locale Constraint #12984

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 6 commits into from

Conversation

zairigimad
Copy link
Contributor

@zairigimad zairigimad commented Jan 24, 2020

Add Canonicalize option to Locale Constraint (which was added in Symfony 4.1: https://symfony.com/index.php/blog/new-in-symfony-4-1-validator-improvements)

@OskarStark
Copy link
Contributor

@ro0NL can you please have a look here? Thanks!

Copy link
Contributor

@ro0NL ro0NL left a comment

Choose a reason for hiding this comment

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

zairigimad and others added 3 commits January 25, 2020 13:37
Co-Authored-By: Oskar Stark <oskarstark@googlemail.com>
Co-Authored-By: Oskar Stark <oskarstark@googlemail.com>
@zairigimad zairigimad requested a review from OskarStark January 25, 2020 12:40
@OskarStark
Copy link
Contributor

For 3.4 I guess?

@zairigimad zairigimad changed the base branch from 5.0 to 3.4 January 25, 2020 13:24
@zairigimad zairigimad changed the base branch from 3.4 to 5.0 January 25, 2020 13:25
@@ -17,6 +17,7 @@ Applies to :ref:`property or method <validation-property-target>`
Options - `groups`_
- `message`_
- `payload`_
_ `canonicalize`_
Copy link
Contributor

Choose a reason for hiding this comment

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

@javiereguiluz shall we sort them in alpha order?

@javiereguiluz javiereguiluz added this to the 4.4 milestone Feb 3, 2020
@javiereguiluz
Copy link
Member

I remembered that this option was already documented. It was merged here: https://github.com/symfony/symfony-docs/pull/9248/files . So I don't understand how/when it was lost 🤔

zairigimad and others added 2 commits February 3, 2020 13:41
Co-Authored-By: Oskar Stark <oskarstark@googlemail.com>
@OskarStark
Copy link
Contributor

@javiereguiluz are you able to finalize this PR?

@wouterj
Copy link
Member

wouterj commented Feb 9, 2020

This option was introduced in 4.1 by symfony/symfony#26075 as a backwards compatibility layer. In 4.1 setting it to anything different than true was deprecated by symfony/symfony#26075 and this setting was removed in Symfony 5.0 (keeping a setting that is only allowed to be true is non-sense) in symfony/symfony#31898

We then correctly also removed this option from the Symfony 5.0+ docs in
ce64ac2#diff-08df604c59d5284048fac6277839ca0dL92

To me, it seems perfectly fine that this option is not documented in 5.0 and master. In the 4.4 version of the docs, this option is still visible: https://symfony.com/doc/4.4/reference/constraints/Locale#canonicalize

For these reasons, I'm closing this PR.

@wouterj wouterj closed this Feb 9, 2020
@ro0NL
Copy link
Contributor

ro0NL commented Feb 10, 2020

@wouterj where did we remove this in 5.x?

https://github.com/symfony/symfony/blob/332fa65f69d2cf929b9f3f07a64c2bd236995c58/src/Symfony/Component/Validator/Constraints/Locale.php#L33

🤔

AFAIK only the default changed in between versions.

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.

6 participants