Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form] Fix inconsistencies#32935
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
Uh oh!
There was an error while loading.Please reload this page.
0f6f833 to2e95887CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Form/ChoiceList/Factory/ChoiceListFactoryInterface.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
8d3ffef todf57d08Comparevudaltsov commentedAug 5, 2019
@Tobion , I reverted all controversial changes. |
src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
vudaltsov commentedAug 5, 2019
@nicolas-grekas , done. |
src/Symfony/Component/Form/ChoiceList/Factory/ChoiceListFactoryInterface.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
b9af18f to360711cComparenicolas-grekas commentedAug 5, 2019
Thank you@vudaltsov. |
This PR was squashed before being merged into the 3.4 branch (closes#32935).Discussion----------[Form] Fix inconsistencies| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | n/a| License | MIT| Doc PR | n/a- ~~Use `@inheritdoc` in `Button` and `ButtonBuilder` where the method does satisfy the contract.~~- ~~Add `This method should not be invoked` in all unsupported methods in `Button` and `ButtonBuilder` for consistency.~~- ~~Fix the misused `idempotent` term in implementations of the `getFormConfig` method. It is wrong in the sense that the method does not always return the same result. You can `setAttribute` for instance and `getFormConfig` will return a different config object.~~- ~~Add `if ($this->locked)` checks in the supported mutators.~~- ~~Fix the arguments contract in the `ChoiceListFactoryInterface` — now it supports `PropertyPathInterface` explicitly. The downside of it — breaking LSP in the `DefaultChoiceListFactory`.~~- Fix the `$label` phpdoc of the `ChoiceView` (arised in#32237).- Use `PropertyPathInterface` instead of `PropertyPath` in `PropertyAccessDecorator` of the choice factory.- Fix `ArrayChoiceList::flatten` type hints.These changes are debatable, so feel free to correct me if I am wrong at some point.Ping@xabbuh ,@HeahDude ,@yceruto ,@nicolas-grekasCommits-------360711c [Form] Fix inconsistencies
Uh oh!
There was an error while loading.Please reload this page.
Use@inheritdocinButtonandButtonBuilderwhere the method does satisfy the contract.AddThis method should not be invokedin all unsupported methods inButtonandButtonBuilderfor consistency.Fix the misusedidempotentterm in implementations of thegetFormConfigmethod. It is wrong in the sense that the method does not always return the same result. You cansetAttributefor instance andgetFormConfigwill return a different config object.Addif ($this->locked)checks in the supported mutators.Fix the arguments contract in theChoiceListFactoryInterface— now it supportsPropertyPathInterfaceexplicitly. The downside of it — breaking LSP in theDefaultChoiceListFactory.$labelphpdoc of theChoiceView(arised in[Form] Add parameter type declarations #32237).PropertyPathInterfaceinstead ofPropertyPathinPropertyAccessDecoratorof the choice factory.ArrayChoiceList::flattentype hints.These changes are debatable, so feel free to correct me if I am wrong at some point.
Ping@xabbuh ,@HeahDude ,@yceruto ,@nicolas-grekas