Refactor the HttplugExtension #97
Conversation
|
cool, thanks! https://github.com/php-http/HttplugBundle/blob/master/Tests/Unit/DependencyInjection/HttplugExtensionTest.php is not a comprehensive test, but it looks like at least the basics have no changes. i think this is going in the right direction. are you confident with it, should we merge it? |
|
Im confident, but.. I was also confident with 1.2.0. If it works when you try it Im happy to merge. More PRs will come up with more test code. |
|
I've rebased this PR so we include #98. That would make me more confident. |
|
can you rebase so that we see if the new tests succeed? |
|
ups, faster than me :-) @sagikazarmark LGTM for me, can you have a look too? |
| } elseif (is_callable($factory)) { | ||
| $client = $factory($config); | ||
| } else { | ||
| throw new \RuntimeException(sprintf('Second argument to PluginClientFactory::createPluginClient must be a "%s" or a callale.', ClientFactory::class)); |
| } | ||
| } | ||
| // Tell the plugin journal what plugins we used | ||
| $container->getDefinition('httplug.collector.plugin_journal') |
There was a problem hiding this comment.
IMO this is still toolbar dependent. Service only declared in data_collector.xml?
Separating profiling into a separate class does not make the code cleaner or more readable. In this commit: - Profiling moved back to main extension - Toolbar (profiling) defaults to kernel.debug
Merge Profiling back to main extension
|
👍 I like this. This could be merged into master. |
I would add this to the changelog |
What's in this PR?
Since the bugs discovered in our extension class I started to refactor it so it would be easier to maintain and understand. This PR will fix #95 or at least be a start of it.
Major changes:
autotoolbar option has been deprecated, defaults to kernel debug