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

[Validator] Add `canonicalize` option for `Locale` validator #22353

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@phansys
Contributor

phansys commented Apr 9, 2017

Q A
Branch master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Allow non canonicalized locales ('fr-FR' by instance) to pass the validation.
Relates to symfony/symfony-docs#7660.

@phansys phansys changed the title from Add `canonicalize` option for `Locale` validator to [Validator] Add `canonicalize` option for `Locale` validator Apr 9, 2017

@nicolas-grekas nicolas-grekas modified the milestone: 3.4 Apr 10, 2017

/**
* {@inheritdoc}
*/
public function __construct($options = null)

This comment has been minimized.

@Koc

Koc Aug 2, 2017

Contributor

Do we need constructor with custom code here?

/**
* {@inheritdoc}
*/
public function getDefaultOption()

This comment has been minimized.

@Koc

Koc Aug 2, 2017

Contributor

Does this method relates to validator class?

@xabbuh

This comment has been minimized.

Member

xabbuh commented Sep 15, 2017

@phansys Can you address @Koc's comments and then also rebase on the 3.4 branch?

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Oct 8, 2017

Moving to 4.1. Rebase on master might be needed, where PHP 7.1 features can be used btw.

@nicolas-grekas nicolas-grekas modified the milestones: 3.4, 4.1 Oct 8, 2017

@phansys

This comment has been minimized.

Contributor

phansys commented Feb 6, 2018

@Koc, sorry for the delay. Your comments were already addressed.
Ping @xabbuh, @nicolas-grekas.

@fabpot

fabpot approved these changes Feb 7, 2018

@fabpot

This comment has been minimized.

Member

fabpot commented Feb 7, 2018

Thank you @phansys.

@fabpot fabpot closed this in 5f0c279 Feb 7, 2018

@fabpot

This comment has been minimized.

Member

fabpot commented Feb 7, 2018

Thinking about this feature (after merging it), would it make sense to always canolicalize the value instead of making it configurable?

@phansys phansys deleted the phansys:locale_validator branch Feb 7, 2018

@phansys

This comment has been minimized.

Contributor

phansys commented Feb 7, 2018

IMO, it would be really useful to allow this validator to always canonicalize the input data, but I think this behavior will break the current "strictness" and thus, that change should be considered a BC break.

@fabpot

This comment has been minimized.

Member

fabpot commented Feb 7, 2018

@phansys Can you work on a PR to generate a deprecation message when canoicalize is false? That way, we are BC and we can remove the option in 5.0.

@phansys

This comment has been minimized.

Contributor

phansys commented Feb 7, 2018

Sure @fabpot, I'll be working on it 👍

symfony-splitter pushed a commit to symfony/validator that referenced this pull request Feb 20, 2018

feature #26075 [Validator] Deprecate use of `Locale` validation const…
…raint without setting "canonicalize" option to `true` (phansys)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Validator] Deprecate use of `Locale` validation constraint without setting "canonicalize" option to `true`

|Q            |A        |
|---          |---      |
|Branch       |master   |
|Bug fix?     |no       |
|New feature? |no       |
|BC breaks?   |no       |
|Deprecations?|yes      |
|Tests pass?  |yes      |
|Fixed tickets|#22353   |
|License      |MIT      |
|Doc PR       |symfony/symfony-docs#9248|

See symfony/symfony#22353 (comment).

Commits
-------

1572540a3a Deprecate use of `Locale` validation constraint without setting "canonicalize" option to `true`

fabpot added a commit that referenced this pull request Feb 20, 2018

feature #26075 [Validator] Deprecate use of `Locale` validation const…
…raint without setting "canonicalize" option to `true` (phansys)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Validator] Deprecate use of `Locale` validation constraint without setting "canonicalize" option to `true`

|Q            |A        |
|---          |---      |
|Branch       |master   |
|Bug fix?     |no       |
|New feature? |no       |
|BC breaks?   |no       |
|Deprecations?|yes      |
|Tests pass?  |yes      |
|Fixed tickets|#22353   |
|License      |MIT      |
|Doc PR       |symfony/symfony-docs#9248|

See #22353 (comment).

Commits
-------

1572540 Deprecate use of `Locale` validation constraint without setting "canonicalize" option to `true`

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Feb 23, 2018

minor #9248 [Validator] Add docs for "canonicalize" option at `Locale…
…` validation constraint (phansys, javiereguiluz)

This PR was merged into the master branch.

Discussion
----------

[Validator] Add docs for "canonicalize" option at `Locale` validation constraint

Related to symfony/symfony#22353, symfony/symfony#26075.

Closes #7660.

Commits
-------

14f10bc Final changes
4c28e1f Minor changes
0cc4ee8 Add docs for "canonicalize" option at `Locale` validation constraint

@fabpot fabpot referenced this pull request May 7, 2018

Merged

Release v4.1.0-BETA1 #27181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment