Skip to content

[Debug] Swap dumper services at bootstrap #19647

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
Aug 23, 2016

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Aug 17, 2016

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

This commit fix a bug when using debug function too soon.
For example, if you call dump function during kernel::boot() the
dump output will be sent to stderr, even in a web context.

With this patch, the data collector is used by default, so the
dump output is send to the WDT. In a CLI context, if dump is used
too soon, the datacollector will buffer it, and release it at the
end of the script. So in this case everything will be visible by the
end used.

This commit fix a bug when using debug function too soon.
For example, if you call dump function during kernel::boot() the
dump output will be sent to stderr, even in a web context.

With this patch, the data collector is used by default, so the
dump output is send to the WDT. In a CLI context, if dump is used
too soon, the datacollector will buffer it, and release it at the
end of the script. So in this case everything will be visible by the
end used.
@nicolas-grekas
Copy link
Member

I think you can go one step further and dump the collected bootstrap dumps in the event listener, by forwarding them to the dumper that is injected there.

@lyrixx
Copy link
Member Author

lyrixx commented Aug 17, 2016

It's not easy because I need to add new "features" into the DumpDataCollector.

So I propose to merge this one, and I will open a new PR for master with your feature.

@nicolas-grekas
Copy link
Member

👍

@nicolas-grekas
Copy link
Member

Thank you @lyrixx.

@nicolas-grekas nicolas-grekas merged commit d80589c into symfony:2.7 Aug 23, 2016
nicolas-grekas added a commit that referenced this pull request Aug 23, 2016
This PR was merged into the 2.7 branch.

Discussion
----------

[Debug] Swap dumper services at bootstrap

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

---

This commit fix a bug when using debug function too soon.
For example, if you call dump function during kernel::boot() the
dump output will be sent to stderr, even in a web context.

With this patch, the data collector is used by default, so the
dump output is send to the WDT. In a CLI context, if dump is used
too soon, the datacollector will buffer it, and release it at the
end of the script. So in this case everything will be visible by the
end used.

Commits
-------

d80589c [Debug] Swap dumper services at bootstrap
@lyrixx lyrixx deleted the debug-invert branch August 23, 2016 08:23
This was referenced Sep 2, 2016
nicolas-grekas added a commit that referenced this pull request Aug 9, 2017
…r wiring (ogizanagi)

This PR was merged into the 2.7 branch.

Discussion
----------

[DebugBundle] Reword an outdated comment about var dumper wiring

| Q             | A
| ------------- | ---
| Branch?       | 2.7 <!-- see comment below -->
| Bug fix?      | no
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | no
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

This comment is outdated since #19647, as the default config is now the one used all the way through in HTTP mode, while it's overridden in CLI mode by the `DumpListener` on `console.command` event.

Commits
-------

f876fd9 [DebugBundle] Reword an outdated comment about var dumper wiring
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.

3 participants