Skip to content

Fix debug tool registration #96

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

Merged
merged 1 commit into from
Jul 19, 2016
Merged

Fix debug tool registration #96

merged 1 commit into from
Jul 19, 2016

Conversation

sagikazarmark
Copy link
Member

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
License MIT

What's in this PR?

Fixes the bundle in production mode.

Why?

In production mode (and when debug mode is disabled) debug tools should not be registered in the clients, otherwise we get weird service not found errors.


### Fixed

- Do not register debug tools debugging is disabled (eg. in prod mode)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...debug tools when debugging is...

@dbu
Copy link
Collaborator

dbu commented Jul 19, 2016

good job, looks right to me. did you try it out?

wonder if we could (not in this PR, but in general) add integration tests for both dev and prod mode to make sure we don't break this again.

Debug collectors and tools like that should not be configured
when debug mode is disabled.
@sagikazarmark sagikazarmark merged commit a318500 into master Jul 19, 2016
@sagikazarmark sagikazarmark deleted the fix_toolbar branch July 19, 2016 11:40
@sagikazarmark
Copy link
Member Author

Yes we should, along with this: #95

@dbu
Copy link
Collaborator

dbu commented Jul 19, 2016

didn't this PR fix that the toolbar is enabled in prod mode?

@sagikazarmark
Copy link
Member Author

$toolbar = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug');

This is the code that decides whether we have a toolbar or not

@sagikazarmark
Copy link
Member Author

The problem was that this configuration was not applied in all cases when some debug service was registered/linked.

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

Successfully merging this pull request may close these issues.

2 participants