-
-
Notifications
You must be signed in to change notification settings - Fork 344
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved support for symfony3 #614
Conversation
a92e1f5
to
976be66
Compare
break; | ||
} | ||
} | ||
|
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.
Maybe use an inflected here?
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.
Inflector damn autocorrect
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 please elaborate?
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.
There are classes to convert snake case to camel case and so on. Doctrine inflector or maybe sf has its own 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.
Not sure if we can improve this with an inflector class. We can't convert everything from snake to camel case.
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.
Then maybe use an associative array type => class?
976be66
to
321e77c
Compare
@@ -66,7 +66,12 @@ public function getDefaultOptions() | |||
*/ | |||
public function getRenderSettings() | |||
{ | |||
return array('sonata_type_filter_default', array( | |||
// NEXT_MAJOR: Remove this line when drop Symfony <2.8 support | |||
$type = method_exists('Symfony\Component\Form\AbstractType', 'getBlockPrefix') |
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 we move this to a method in abstract class Filter extends BaseFilter
?
in this case we avoid code duplication for the default stuff
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.
👎 Introducing a method only for BC handling is a bad idea
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.
ok, fine for me
lets merge this as it is
Thank you @core23 👍 |
This is not a minor but a patch. |
I am targetting this branch, because…
Changelog
Subject
This will reduce some deprecation warnings and supports the new class usage for forms.