-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
[Debug] added the component (closes #6828, closes #6834, closes #7330) #7441
Conversation
This is just a quick start to begin the discussion. |
}, | ||
"autoload": { | ||
"psr-0": { "Symfony\\Component\\Debug\\": "" } | ||
}, |
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.
It need to suggest (and probably require-dev as well) HttpFoundation (required to use the ExceptionHandler) and PsrLog (optionally used by the ErrorHandler to log deprecations)
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, I did not added anything yet as this is just to start the discussion. And dependencies is indeed one topic.
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.
Would it be possible to eliminate the dependency to HttpFoundation to make it standalone ?
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.
In fact, it is not required but if you have it, you have some extra features. The same goes with HttpKernel (and the special handling of HTTP exception classes).
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 ExceptionHandler has a hard dependency on the Response class. Its handle method is $this->createResponse($exception)->send();
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.
That's not a hard dep as you can get the content and the css directly.
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 response is not a hard dependency of the FlattenException, this is true. But it is a hard dependency of the ExceptionHandler. You cannot register the exception handler without HttpFoundation
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.
well, it is a hard dependency if we want to use it as an exception handler (which is its goal)
Apart from my comments,, this looks good to me. and it is great to have the deprecation handling standalone. It means it is possible to trigger E_USER_DEPRECATED in libraries when deprecating something and users can get the handling without having to use HttpKernel. 👍 |
@@ -34,6 +34,16 @@ UPGRADE FROM 2.x to 3.0 | |||
* `Symfony\Bridge\Monolog\Logger` | |||
* `Symfony\Component\HttpKernel\Log\NullLogger` | |||
|
|||
* The `Symfony\Component\HttpKernel\Kernel::init()` method has been removed. |
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.
Could this be reconsidered (or at least rediscussed)? maybe in a separate PR (as this is not a blocking step for creating this standalone component)?
@fabpot : your last commits are awesome, I have no objection anymore :) |
composer.lock | ||
phpunit.xml | ||
Tests/ProjectContainer.php | ||
Tests/classes.map |
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.
These are not necessary AFAIK. They are left-over of the HttpKernel copy-paste
* If the Symfony ClassLoader component is available, a special | ||
* class loader is also registered. | ||
* | ||
* @param integer $errorReportingLevel The level of error reporting you wan |
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.
want
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.
fixed
This PR was merged into the master branch. Discussion ---------- [Debug] added the component (closes #6828, closes #6834, closes #7330) | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #6828, #6834, #7330 | License | MIT | Doc PR | symfony/symfony-docs#2479 You can use the individual tools, or register them all: ```php use Symfony\Component\Debug\Debug; Debug::enable(); ``` Changes in Symfony SE: symfony/symfony-standard#523 Commits ------- f693128 fixed typos 1ab1146 [Debug] fixed minor bugs daa3a3c [Debug] changed composer to accept more versions e455269 [Debug] ensured that the Debug tools can only be registered once 946bfb2 [Debug] made the exception handler independant of HttpFoundation 2b305c2 added a main Debug class to ease integration 2ff0927 [Debug] added the component (closes #6828, closes #6834, closes #7330)
You can use the individual tools, or register them all:
Changes in Symfony SE: symfony/symfony-standard#523