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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpKernel][WebProfilerBundle] Profile stack #32491

Open
wants to merge 1 commit into
base: 4.4
from

Conversation

Projects
None yet
3 participants
@fancyweb
Copy link
Contributor

commented Jul 10, 2019

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

So I did this quickly to get feedback. It could probably be splitted for clarity.

The goal of this work is to provide a better DX with Ajax requests in the WDT.

Having a private profile stack allow us to pass it to the WebDebugToolbarListener to process collected data and to directly build the profiler link with the useful panel.

I think that adding a direct link to the dumps panel when there are dumps in an Ajax request would be very useful.

It would also impove the work done in #26665 by being sure there is actually an exception in the exception panel.

Since all impacted classes were made @final in 4.3, it is easy to drop the current behavior (ie remove the double usage of the $profiles / ProfileStack in the ProfilerListener and the optional ProfileStack in the WebDebugToolbarListener) and to fully switch to the new one in 5.0.

馃憤 or 馃憥 ?

@fancyweb fancyweb force-pushed the fancyweb:profile-stack branch from bdad285 to dff1130 Jul 10, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone Jul 11, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

Can you please share a screenshot (with a short animation if that makes sense?) This always helps a lot when improving the profiler.

@fancyweb

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

Sure, considering there is a dump in the code :

Currently :
screenshot_01

After :
screenshot_03

If there is a dump, it's very likely that the user wants to see it. It saves one click.

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