Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Debug] Mark ErrorHandler and ExceptionHandler classes as final#28954
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
[Debug] Mark ErrorHandler and ExceptionHandler classes as final#28954
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedOct 22, 2018
What about making the whole class final? That'd be even better to me :) |
Uh oh!
There was an error while loading.Please reload this page.
fancyweb commentedOct 23, 2018
Making the whole class final is more protective and might indeed be better for future similar problems. In this case we should mark the |
nicolas-grekas commentedOct 23, 2018
Would work for me. |
32e2320 to2a4e2e6Compare| 4.3.0 | ||
| ----- | ||
| * made the`ErrorHandler` and`ExceptionHandler` classes final |
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.
They aren't actually final, just marked as such. It would make more sense in my opinion, to write "marked the ..."
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.
Yeah I thought about this but I actually searched how these kind of deprecations were handled in the past in the changelogs, and they were done like that 😕
Should I add also add an entry for the 5.0.0 with the fact that these classes will really be final at this point ?
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.
@final since Symfony 4.3 so the changelog is correct. Bothfinal class and@final are final :)
nicolas-grekas commentedDec 1, 2018
Thank you@fancyweb. |
… as final (fancyweb)This PR was merged into the 4.3-dev branch.Discussion----------[Debug] Mark ErrorHandler and ExceptionHandler classes as final| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | not yet| Deprecations? | yes| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -The goal of marking this method final is to be able to change the argument signature to `\Throwable` in Symfony 5.0We will then be able to convert the incoming `\Throwable` to `\ErrorException` thanks to the `FatalThrowableError` class.The use case is when you use the `ExceptionHandler::register()` method of the `Debug` component with a custom `set_error_handler()` that don't handle this conversion. This is for example the case of the `Drupal` one.Commits-------2a4e2e6 [Debug] Mark the ErrorHandler and ExceptionHandler classes as final
The goal of marking this method final is to be able to change the argument signature to
\Throwablein Symfony 5.0We will then be able to convert the incoming
\Throwableto\ErrorExceptionthanks to theFatalThrowableErrorclass.The use case is when you use the
ExceptionHandler::register()method of theDebugcomponent with a customset_error_handler()that don't handle this conversion. This is for example the case of theDrupalone.