Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged

Conversation

@vudaltsov
Copy link
Contributor

@vudaltsovvudaltsov commentedAug 4, 2019
edited
Loading

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a
  • Use@inheritdoc inButton andButtonBuilder where the method does satisfy the contract.
  • AddThis method should not be invoked in all unsupported methods inButton andButtonBuilder for consistency.
  • Fix the misusedidempotent term in implementations of thegetFormConfig method. It is wrong in the sense that the method does not always return the same result. You cansetAttribute for instance andgetFormConfig will return a different config object.
  • Addif ($this->locked) checks in the supported mutators.
  • Fix the arguments contract in theChoiceListFactoryInterface — now it supportsPropertyPathInterface explicitly. The downside of it — breaking LSP in theDefaultChoiceListFactory.
  • Fix the$label phpdoc of theChoiceView (arised in[Form] Add parameter type declarations #32237).
  • UsePropertyPathInterface instead ofPropertyPath inPropertyAccessDecorator of the choice factory.
  • FixArrayChoiceList::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-grekas

@vudaltsovvudaltsovforce-pushed theform-inconsistencies-fixes branch from0f6f833 to2e95887CompareAugust 4, 2019 22:10
@nicolas-grekasnicolas-grekas added this to the3.4 milestoneAug 5, 2019
@vudaltsovvudaltsovforce-pushed theform-inconsistencies-fixes branch from8d3ffef todf57d08CompareAugust 5, 2019 10:16
@vudaltsov
Copy link
ContributorAuthor

@Tobion , I reverted all controversial changes.

@vudaltsov
Copy link
ContributorAuthor

@nicolas-grekas , done.
Also removed to useless phpdoc types inChoiceListFactoryInterface

@nicolas-grekasnicolas-grekasforce-pushed theform-inconsistencies-fixes branch fromb9af18f to360711cCompareAugust 5, 2019 14:17
@nicolas-grekas
Copy link
Member

Thank you@vudaltsov.

@nicolas-grekasnicolas-grekas merged commit360711c intosymfony:3.4Aug 5, 2019
nicolas-grekas added a commit that referenced this pull requestAug 5, 2019
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
@vudaltsovvudaltsov deleted the form-inconsistencies-fixes branchAugust 5, 2019 14:18
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@xabbuhxabbuhxabbuh left review comments

@ycerutoycerutoyceruto approved these changes

+1 more reviewer

@TobionTobionTobion requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

6 participants

@vudaltsov@nicolas-grekas@Tobion@xabbuh@yceruto@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp