Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Validator][Choice] Make strict the default option for choice validation#19257
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
stof commentedJul 1, 2016
Such BC break cannot be done in 3.x |
peterrehm commentedJul 1, 2016
@stof I was unsure about that, as for me this is a serious bug in the current behaviour. If that is not possible the only way would be option 2 and 3 where I would prefer 2. |
ro0NL commentedJul 1, 2016
Could this target 4.x? Not sure this is common.. but master could trigger a deprecation error about changing behavior in 4.x right? |
peterrehm commentedJul 1, 2016
master could introduce the deprecation, so option 2 or 3 should be possible. |
ro0NL commentedJul 1, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yeah, but option 1 would be ideal right? I.e. option 1 & 2 are practically the same, it just proposes to rename the option.. which isnt necessarily needed imo. |
peterrehm commentedJul 1, 2016
Yes, it would be great to fix it without the rename, lets see. |
nicolas-grekas commentedJul 10, 2016
Options 2 or 3 are the only viable solutions, to preserve BC and enable a continuous upgrade path. |
xabbuh commentedJul 10, 2016
If noone has a good use case for being able to switch the behaviour of the constraint, I would deprecate setting the |
peterrehm commentedJul 10, 2016
Updated accordingly. The open decision is if we want to deprecate the entire option or just the default value and as later still allow setting it to false. |
peterrehm commentedJul 13, 2016
Should be good to go from my end now. |
peterrehm commentedAug 8, 2016
Any further feedback? |
xabbuh commentedAug 8, 2016
Can you update the upgrade file for 4.0 too please? Besides that the changes look good to me. |
peterrehm commentedAug 8, 2016
Updated. Besides that the BC Break can be removed as the current solution does not contain any BC break. |
fabpot commentedSep 14, 2016
Thank you@peterrehm. |
… choice validation (peterrehm)This PR was squashed before being merged into the 3.2-dev branch (closes#19257).Discussion----------[Validator][Choice] Make strict the default option for choice validation| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |#18973| License | MIT| Doc PR | -This is just the WIP as there are two options.1. Just change default which would only possible to introduce in 4.x or in 3.2 if this BC break is considered as acceptable2. Add a new option e.g. `strictComparison` which defaults to true in 4.x and deprecate the usage of the strict option for 3.2.3. Just deprecate strict = false and remove the option but I would be against that as we remove flexibility which might be wanted.As per discussion I went ahead with option 3. We can then still decide if we want to remove the option entirely or eventually reenable setting strict to false in a later release.Commits-------177c513 [Validator][Choice] Make strict the default option for choice validation
Strate commentedJan 22, 2018
How can I avoid deprecation warning without adding |
Uh oh!
There was an error while loading.Please reload this page.
This is just the WIP as there are two options.
strictComparisonwhich defaults to true in 4.x and deprecate the usage of the strict option for 3.2.As per discussion I went ahead with option 3. We can then still decide if we want to remove the option entirely or eventually reenable setting strict to false in a later release.