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

Adds possibility to override MonologAdapter in config #19

Closed
wants to merge 1 commit into from

Conversation

vokrik
Copy link

@vokrik vokrik commented Sep 19, 2016

You can now specify your own Adapter in the config.neon, in which you can for example opt out from having exceptions saved to a file or add more context to your error messages (such as current Request etc.)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.952% when pulling 652b0f2 on vokrik:master into 9b6ca9c on Kdyby:master.

@vokrik vokrik changed the title Adds possibility to override MonologExtension in config Adds possibility to override MonologAdapter in config Sep 19, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.952% when pulling 3b66ae3 on vokrik:master into 9b6ca9c on Kdyby:master.

@vokrik
Copy link
Author

vokrik commented Sep 20, 2016

Sorry for the force push, forgot to change visibility from private to protected

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.952% when pulling 6e1d621 on vokrik:master into 9b6ca9c on Kdyby:master.

@@ -41,7 +41,7 @@ class MonologAdapter extends Logger
/**
* @var Monolog\Logger
*/
private $monolog;
protected $monolog;
Copy link
Member

Choose a reason for hiding this comment

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

this is not needed, since you can catch the constuctor argument in the inherited class

@@ -75,7 +76,7 @@ public function loadConfiguration()

// Tracy adapter
$builder->addDefinition($this->prefix('adapter'))
->setClass('Kdyby\Monolog\Diagnostics\MonologAdapter', [$this->prefix('@logger')])
->setClass($config['adapter'], [$this->prefix('@logger')])
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK you can already do this using services section of neon configuration, no?

@fprochazka
Copy link
Member

You can now specify your own Adapter in the config.neon

I believe you can do so using the services section already.

in which you can for example opt out from having exceptions saved to a file

If there is a real need for this, there should be a switch in extension configuration. Otherwise, the whole point of this package is to mainly add monolog to tracy and keep bluescreen rendering working. So I'm not sure it really is needed.

or add more context to your error messages (such as current Request etc.)

That should be done with registering a custom processor.

Sorry for the force push, forgot to change visibility from private to protected

Forcepushes are completely fine in pull requests.

@fprochazka
Copy link
Member

If some of the problems mentioned are still an issue for you, let's solve them one by one. I don't think the proposed way is a good solution for any of those mentioned problems. But thank you for the PR!

@fprochazka fprochazka closed this May 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants