Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Mark all dispatched event classes as final#33152
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
| * @author Fabien Potencier <fabien@symfony.com> | ||
| */ | ||
| class MessageEventextends Event | ||
| finalclass MessageEventextends Event |
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.
@fabpot there is aMessageEvents class in the same mailer namespace. This can easily be confused with the event class and should maybe be renamed or relocated or even made internal.
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.
isn't MessageEvents the class holding all event names as constants ? This is not internal
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.
The Mailer MessageEvents is a class used for logging and data collecting. It collects events but is not an event itself.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…so we can make it final (Tobion)This PR was merged into the 4.3 branch.Discussion----------[HttpKernel] Do not extend the new SF 4.3 ControllerEvent so we can make it final| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | unlikely| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets || License | MIT| Doc PR |See#33152 (comment)Remember the ControllerEvent is new in SF 4.3 so we just go back to what it was before 4.3Commits-------00140b6 Do not extend the new SF 4.3 ControllerEvent so we can make it final
nicolas-grekas commentedAug 21, 2019
Thank you@Tobion. |
This PR was merged into the 4.4 branch.Discussion----------Mark all dispatched event classes as final| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets || License | MIT| Doc PR |I think we should mark all our Event classes as final. There is no point in people extending them as the libraries that use the event, will only dispatch this event. So extending events in user-land achieves nothing as the subclasses won't be dispatched.I'm not talking about the base events that are meant to be extended like KernelEvent, but the leaf events like ExceptionEvent, ResponseEvent etc.Then we can also make them real final in 5.0 as the events are value objects that should not be mocked.Commits-------4bb38ee Mark all dispatched event classes as final
This PR was merged into the 5.0-dev branch.Discussion----------Make dispatched events really final| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets || License | MIT| Doc PR |Continuation of#33152 for masterCommits-------953057f Make dispatched events really final
I think we should mark all our Event classes as final. There is no point in people extending them as the libraries that use the event, will only dispatch this event. So extending events in user-land achieves nothing as the subclasses won't be dispatched.
I'm not talking about the base events that are meant to be extended like KernelEvent, but the leaf events like ExceptionEvent, ResponseEvent etc.
Then we can also make them real final in 5.0 as the events are value objects that should not be mocked.