Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[2.8] [Form] Rename CollectionType options for entries#15025
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
| @@ -52,12 +52,12 @@ class ResizeFormListener implements EventSubscriberInterface | |||
| */ | |||
| private $deleteEmpty; | |||
| public function __construct($type, array $options = array(), $allowAdd = false, $allowDelete = false, $deleteEmpty = false) | |||
| public function __construct($entryType, array $entryOptions = array(), $allowAdd = false, $allowDelete = false, $deleteEmpty = false) | |||
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.
Can you revert the variable renames in this method? They don't really make sense now IMO.
We need a few legacy tests using the old option names |
@wouterj We need to finish this one this week if we want to include it in 2.8. |
@fabpot I'm sorry for the delay. fixed all comments & added some legacy tests. Should be ready to merge now |
| @@ -791,7 +791,7 @@ public function testSubmitSingleExpandedRequiredEmptyNoChoices() | |||
| $this->assertTrue($form->isSynchronized()); | |||
| } | |||
| public functiontestSubmitSingleExpandedRequiredFalse() | |||
| public functiontestSubmiCollectiontSingleExpandedRequiredFalse() | |||
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.
missingt ofSubmit
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.
This change seems like a mistake, reverted it
I cannot relate any of the build failures to changes in this PR |
Thank you@wouterj. |
… (WouterJ)This PR was merged into the 2.8 branch.Discussion----------[2.8] [Form] Rename CollectionType options for entriesDescription---Replaces#13820 for the 2.8 branch.Original description: > `type` and `options` are extremely generic. Prefixing them with `entry_` makes it clear what they are configuring. > About the property deprecation it is the same story as#13717 and I don't know which direction you want me to go.I've tried to apply the comments in the previous PR, but got a bit lost in the normalizers/default closure stuff. I hope I did everything correctly, but please review :)PR Info Table---| Q | A| ------------- | ---| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |#7831| License | MIT| Doc PR |symfony/symfony-docs#5051Commits-------942a237 Rename CollectionType options for entries
I'm using |
This PR was submitted for the 2.7 branch but it was merged into the 2.8 branch instead (closes#5051).Discussion----------Rename CollectionType entry options| Q | A| --- | ---| Doc fix? | no| New docs? | yes (symfony/symfony#15025)| Applies to | 2.7+| Fixed tickets | -Commits-------1b00278 Rename CollectionType entry options
Description
Replaces#13820 for the 2.8 branch.
Original description:
I've tried to apply the comments in the previous PR, but got a bit lost in the normalizers/default closure stuff. I hope I did everything correctly, but please review :)
PR Info Table