-
Notifications
You must be signed in to change notification settings - Fork 179
Exposes Sentry\Monolog\Handler service for easier monolog configuration. #225
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
Conversation
Second option of integration is via https://github.com/getsentry/raven-php, but it's marked as deprecated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems an interesting addition, which would create no downsides if it's not used.
I would add some tests to check for the existence of the service definition, and instead of adding that to the examples, I would add that in the readme!
Good idea. Will do that later today. Thanks for looking at this ! |
I've added test You've requested, but it's failing due to a valid reason. If someone would install this bundle without having
Will wait for Your feedback, but I'm leaning towards closing this PR. |
Surely defining that service every time would be bad for sure. Instead of closing, maybe we can reroute this to a README-only PR? You could suggest the service definition there too. |
9c466dc
to
be3aff6
Compare
be3aff6
to
1719c92
Compare
As suggested, I've updated only |
Thanks! |
|
||
```yaml | ||
monolog: | ||
sentry: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need to be
monolog:
handlers:
sentry:
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right! #230
I ❤️ using this bundle.
This PR solves one problem that's still cumbersome to me.
sentry/sentry
ships withSentry\Monolog\Handler
for smooth integration withmonolog
, however this bundle is not exposing it as a service making integration more difficult.My PR configures
Sentry\Monolog\Handler
as a service, which - in theory - should simplify configuration.I've updated example for reference, but it's commented out since not everyone might like it configured out-of-the-box.