-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Such BC break cannot be done in 3.x |
@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. |
Could this target 4.x? Not sure this is common.. but master could trigger a deprecation error about changing behavior in 4.x right? |
master could introduce the deprecation, so option 2 or 3 should be possible. |
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. |
Yes, it would be great to fix it without the rename, lets see. |
Options 2 or 3 are the only viable solutions, to preserve BC and enable a continuous upgrade path. |
If noone has a good use case for being able to switch the behaviour of the constraint, I would deprecate setting the |
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. |
Should be good to go from my end now. |
Any further feedback? |
Can you update the upgrade file for 4.0 too please? Besides that the changes look good to me. |
Updated. Besides that the BC Break can be removed as the current solution does not contain any BC break. |
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 acceptable 2. 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
How can I avoid deprecation warning without adding |
This is just the WIP as there are two options.
strictComparison
which 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.