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][FrameworkBundle] Move FormPass to the Form component#21283
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
4168a0e to123be59Comparedunglas commentedJan 14, 2017
👍 |
| $container->addCompilerPass(newAddConsoleCommandPass()); | ||
| } | ||
| $container->addCompilerPass(newFormPass()); | ||
| if (class_exists(FormPass::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.
As this going to be very big would it make sense to use a private helper method (which only adds when the class exists)?
$this->addExistingCompilerPass($container, $className, $arguments = array()).
So you don't end-up with numerous if-statements 👍
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.
I tend to agree 👍 let's see if deciders think it's worth it
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.
A simple private method in this class can do the trick isn't it?
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.
Sure, method added
2ffe098 to2c91c6dCompare2b57f1b tod6ac6f3Comparechalasr commentedJan 24, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Updated this to make the form extension service id and form type/type_extension/type_guesser tags configurable through the constructor, defaulting to the ones used in the framework. |
d6ac6f3 to2ba70a8Compare2ba70a8 toe68a6d9Comparefabpot commentedFeb 16, 2017
Thank you@chalasr. |
…onent (chalasr)This PR was merged into the 3.3-dev branch.Discussion----------[Form][FrameworkBundle] Move FormPass to the Form component| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets | n/a| License | MIT| Doc PR | n/aSo that anyone using only Form and DI can use it for registering form types/type guessers.Follows#19443, related to#21284Commits-------e68a6d9 [FrameworkBundle][Form] Move FormPass to the Form component
ro0NL commentedFeb 16, 2017
Well.. the approach now differs with#20250 (see discussion#20250 (comment), ie. default args vs. required args) Yet im rethinking it.. maybe having defaults in library code is good for convenience any way, and#20250 needs an update. |
Uh oh!
There was an error while loading.Please reload this page.
So that anyone using only Form and DI can use it for registering form types/type guessers.
Follows#19443, related to#21284