Skip to content

[Debug] deprecate the component in favor of ErrorCatcher #32303

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

Closed
wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 4.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Keeping the Debug and DebugClassLoader classes in a standalone component creates a pile of complications that don't have to be. Let's make things simple and move everything to ErrorCatcher.

Fix symfony/recipes#619
Fix symfony/skeleton#177

@stof
Copy link
Member

stof commented Jul 1, 2019

is DebugClassLoader the only debug utility we need to provide ?

@nicolas-grekas
Copy link
Member Author

I think so. At least we don't have any other.

@lyrixx
Copy link
Member

lyrixx commented Jul 1, 2019

I have just one concern about discoverability. Is this something we should take care of?

@fancyweb
Copy link
Contributor

fancyweb commented Jul 1, 2019

Aren't we regret this in the future if we create new debug utilities? 😕

Also, having the DebugClassLoader in a component that was created to render PHP errors / exceptions in different formats does not seem consistent.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Jul 1, 2019

That's still the most pragmatic way to solve the issues that the linked issues describe.
And having a component with debug utilities is just normal (while having a Debug component with non-debug things is not.)

@fancyweb
Copy link
Contributor

fancyweb commented Jul 1, 2019

But that still doesn't solve the problem for existing projects, right? 🤔 Their index.php is still referencing the Debug component but it's still not required anymore.

@nicolas-grekas
Copy link
Member Author

As with any regular deprecations. The issue is different, see linked issues. (Sorry I can't elaborate I'm on mobile and about to be AFK)

@Tobion
Copy link
Contributor

Tobion commented Jul 1, 2019

I thought the whole point of creating the ErrorCatcher component was to split the debugging part and the part that is required even without debugging turned on.
Otherwise we could have just added the new ErrorRendering inside the debug component (or any other component like HttpFoundation) and wouldn't have to deprecate anything.
So by moving everything into ErrorCatcher, we achieve nothing but have a huge number of deprecations and BC breaks without any gain. Big 👎 for me.

@nicolas-grekas
Copy link
Member Author

The main point was to improve and split the rendering part actually. Splitting the debug tools wouldn't be of much interest to me.
So, in the end, in think we need an ErrorHandler component, that contains the debug tools too, but NO rendering at all.
Then rendering would be split in a dedicated component, and we would deprecate Debug and drop ErrorCatcher.

@yceruto
Copy link
Member

yceruto commented Jul 1, 2019

But that still doesn't solve the problem for existing projects, right? thinking Their index.php is still referencing the Debug component but it's still not required anymore.

@fancyweb we could use the flex command symfony:sync-recipes to update the index.php file with the new namespace, I know, that will require an extra effort and should be informed as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants