Skip to content
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] deprecate the component in favor of ErrorCatcher #32303

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
7 participants
@nicolas-grekas
Copy link
Member

commented Jul 1, 2019

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

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

is DebugClassLoader the only debug utility we need to provide ?

@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2019

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

@lyrixx

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

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

@fancyweb

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member Author

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2019

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2019

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

This comment has been minimized.

Copy link
Member

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.

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:debug-deprec branch Jul 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.