search

jenssegers / laravel-rollbar

Rollbar error monitoring integration for Laravel projects
329 stars 99 forks source link

Use rollbar v1.0.1 #85

Closed olafnorge closed 7 years ago

olafnorge commented 7 years ago
iwex commented 7 years ago

So how to cooperate? :)

olafnorge commented 7 years ago

Hi @iwex,

good to hear from you. I would suggest that we first decide which of the PRs we should proceed with. After that we can write tests to get a better coverage of what we have.

I am emotionless on which PR we proceed, both look promising. Maybe the guys with write access to the repo can help with the decision. And I think we are not in a hurry. :)

What do you think @jenssegers?

brianr commented 7 years ago

Hey folks - there's also a similar effort that @ArturMoczulski (main developer on rollbar-php 1.0) did for the Laravel integration. It's over at https://github.com/rollbar/rollbar-php-laravel and is actually a fork of this library (jenssegers/laravel-rollbar).

@ArturMoczulski is that ready for general consumption?

iwex commented 7 years ago

@brianr I don't think it's ready https://github.com/rollbar/rollbar-php-laravel/blob/master/src/RollbarLogHandler.php#L78 :)

olafnorge commented 7 years ago

It feels for me a little bit confusing and 'chaotic' to have now at least 3 version for the same thing.

ArturMoczulski commented 7 years ago

https://github.com/rollbar/rollbar-php-laravel is the official Laravel support package supported by Rollbar. It's a fork of jenssegers/laravel-rollbar, but that is the repo we will be focusing on moving forward. Yes, it is ready for general consumption. It's not in packagist repos yet, but README.md includes instructions how to composer require from GitHub. It's going to make its' way to packagist very soon.

ArturMoczulski commented 7 years ago

In regards to ::addContext(), rollbar-php-laravel supports it: https://github.com/rollbar/rollbar-php-laravel/blob/master/src/RollbarLogHandler.php#L76

Also, adding Laravel session information is done automatically out of the box: https://github.com/rollbar/rollbar-php-laravel/blob/master/src/RollbarLogHandler.php#L86-L113

olafnorge commented 7 years ago

So then I don't think it makes sense to proceed here. But it was a good chance to learn about how the things work together.

I will close this PR in favour of Arturs. Thank you guys for reviewing and making suggestions.

drbyte commented 7 years ago

@ArturMoczulski your README at https://github.com/rollbar/rollbar-php/blob/master/README.md still tells people to use the jenssegers package, instead of your own rollbar-php-laravel package.

briandotdev commented 7 years ago

The Rollbar docs also still point to this repo for Laravel integration. https://rollbar.com/docs/notifier/rollbar-php/

jarnovanleeuwen commented 7 years ago

@olafnorge So this repository will no longer update its Rollbar dependency? Will this repository be abandoned in favour of https://github.com/rollbar/rollbar-php-laravel, or is this still unclear? If yes, this might be something to add to the readme.

olafnorge commented 7 years ago

@jarnovanleeuwen I can't tell because I am not the maintainer of this repo I just opened a PR and closed it.

jarnovanleeuwen commented 6 years ago

Rollbar officially announced/released its Laravel package (https://rollbar.com/blog/automated-laravel-error-reporting/). Their repo is ahead of this one and I think people should abandon this repo in favour of the official repo that is also relying on the latest rollbar/rollbar dependency.