Fix reference to RollbarHandler #94
Conversation
|
Blows my mind that this hasn't been accepted and merged |
|
Related PR: rollbar/rollbar-php-symfony-bundle#63 |
|
@sonnysavage hey there, new product guy here at Rollbar and, you're right to be surprised. Getting someone to get this reviewed so we can merge. |
|
Hi folks, after a review we're closing this PR. Our rationale. The Monolog dependency has a Rollbar handler built into the package. However, because of that there is a dance that needed to happen when changes were rolled out. If developers only updated rollbar/rollbar-php-laravel and not monolog/monolog they would not get the Monolog\Handler\RollbarHandler class or the right version of that class. It would take a lot more work to track down exactly what combination of rollbar/rollbar-php, monolog/monolog, and rollbar/rollbar-php-laravel would cause issues when combined. However, our belief is this probably not an issue for most if any users since they would need a three year old version of monolog to cause this issue. We are hopeful it is safe to assume that most developers have not had their system in an error state for the last 3 years, or that they can/have run "composer update". All that being said, merging #94 would actually be a step backwards. Monolog\Handler\RollbarHandler is the correct reference and Rollbar\Monolog\Handler\RollbarHandler is the old one. If we are looking for a permanent fix for the issue described in #94, it would require creating a compatibility matrix on the versions that existed 3ish years ago and releasing patches that would keep people from installing incompatible versions. This is probably an edge case at this point. We are be hesitant to do this, since getting it wrong could cause existing builds to fail when applying the patch. Thanks again for your support and apologies on the delays to getting to this "end." |
|
It doesn't make sense to me that Monolog would own the Handler. 🤷🏻 |
See #93