Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Form] Add upgrade path for empty_data option in TextType#41516
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
base:7.4
Are you sure you want to change the base?
[Form] Add upgrade path for empty_data option in TextType#41516
Uh oh!
There was an error while loading.Please reload this page.
Conversation
748c37b to1b38c73CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Having all those test cases marked as legacy looks suspicious to me. Should they all really be removed in 6.0? If not, how should they look like then?
Uh oh!
There was an error while loading.Please reload this page.
e103ebb to4a44fb6Compare
@nicolas-grekas I am trying to solve theproblem in tests. What is the best way to do this?
|
Marking all these tests as legacy is not the way to go. Instead all the tests should be updated the same way an Symfony user would have to update their code to keep the existing behavior when this PR is merged. This will allow us to let the tests show us the impact the proposed change has on our users and we can use it as a base for a discussion about whether or not this is worth it. |
57ea40c to7d9556eCompare| @@ -27,6 +27,9 @@ public function buildForm(FormBuilderInterface $builder, array $options) | |||
| // See https://github.com/symfony/symfony/issues/5906#issuecomment-203189375 | |||
| if ('' === $options['empty_data']) { | |||
| $builder->addViewTransformer($this); | |||
| } elseif (FormType::$emptyData === $options['empty_data']) { | |||
| // "empty_data" option is the default value from FormType::configureOptions() | |||
| trigger_deprecation('symfony/form', '5.4', 'The default value of "empty_data" option in "%s" will be changed to empty string. Declare "NULL" as value for "empty_data" if you still want use "NULL" as data.', __CLASS__); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
To ensure that this is the default closure and not the user-specified one, we must compare the provided closure with the default closure. To do this, the default closure must be created once and be accessible from the outside.
31f105e to52279c4Compare52279c4 toed68472Compareed68472 to2306c3aCompareWhat's the status here? |
643286f tof1c43f9Compare
@fabpot needs review |
a1be371 to2d9849fCompare2d9849f tofa0328dCompare
Uh oh!
There was an error while loading.Please reload this page.
Upgrade path for#41514