Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[EventDispatcher] add method getListenerPriority() to interface#16301
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
xabbuh commentedOct 20, 2015
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | yes |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #16198 |
| License | MIT |
| Doc PR | todo |
dupuchba commentedOct 21, 2015
👍 |
Tobion commentedOct 23, 2015
👍 Status: Reviewed |
Tobion commentedOct 23, 2015
Thank you@xabbuh. |
…interface (xabbuh)This PR was merged into the 3.0-dev branch.Discussion----------[EventDispatcher] add method getListenerPriority() to interface| Q | A| ------------- | ---| Bug fix? | no| New feature? | yes| BC breaks? | yes| Deprecations? | no| Tests pass? | yes| Fixed tickets |#16198| License | MIT| Doc PR | todoCommits-------f8019c8 add method getListenerPriority() to interface
pfrenssen commentedMar 7, 2016
Is there a possibility to backport this to Symfony 2.8? The |
xabbuh commentedMar 7, 2016
@pfrenssen Adding a method to an interface is a BC break (all implementations of the interface must be changed). That's why we had to make this change in 3.0. |
pfrenssen commentedMar 7, 2016
Ok that sounds like a good reason. Pity is that BC is already broken, ContainerAwareEventDispatcher is calling this method in 2.8. |
xabbuh commentedMar 7, 2016
@pfrenssen I do not see where this is happening. But can you please create a new issue if you think that there actually is a bug (always calling a method that is not given by the interface would be indeed a mistake). Best would be if you could come up with a failing test case or code example. |
donquixote commentedMar 30, 2016
@xabbuh@pfrenssen here,https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php#L102
This could be done by adding a separate interface, extending the basic one. The question is: Should TraceableEventDispatcher always require a dispatcher with getListenerPriority() ? Or is this called only sometimes? Or could it return a default value, if the method does not exist?
I leave this to others (e.g.@pfrenssen) For now I am simply having a short visit in this issue queue. |
xabbuh commentedMar 31, 2016
@pfrenssen@donquixote Thank you for the details given. I checked the code again and proposed a fix in#18388 to provide a meaningful exception in case you call the |
pfrenssen commentedMar 31, 2016
I guess this is a good solution if we cannot add the interface for BC reasons. Thanks! |
This PR was merged into the 2.8 branch.Discussion----------[EventDispatcher] check for method to exist| Q | A| ------------- | ---| Branch? | 2.8| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#16301 (comment)| License | MIT| Doc PR |This change must be reverted after being merged into the `3.0` branch (the `getListenerPriority()` method was added to the interface in Symfony 3.0).Commits-------78ae2ad [EventDispatcher] check for method to exist
This PR was merged into the 2.8 branch.Discussion----------[EventDispatcher] check for method to exist| Q | A| ------------- | ---| Branch? | 2.8| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |symfony/symfony#16301 (comment)| License | MIT| Doc PR |This change must be reverted after being merged into the `3.0` branch (the `getListenerPriority()` method was added to the interface in Symfony 3.0).Commits-------78ae2ad [EventDispatcher] check for method to exist
Implement `getListenerPriority()` that was introduced in Symfony 2.8.symfony/symfony#16301