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 Lumen 5.x support #54

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

GiamPy5
Copy link

@GiamPy5 GiamPy5 commented May 23, 2016

Adds Lumen 5.x support by using Monolog\RollbarHandler.

@GiamPy5
Copy link
Author

GiamPy5 commented May 23, 2016

StyleCI analysis has been fixed, it shows as failed because I have used a soft reset to squash the commits. @jenssegers

@GiamPy5 GiamPy5 mentioned this pull request May 24, 2016
@jenssegers
Copy link
Owner

jenssegers commented May 24, 2016

Wouldn't it be better to refactor the original service provider so that the Lumen service provider can extend it and only override Lumen specific stuff?

This way there is less code duplication.

@GiamPy5
Copy link
Author

GiamPy5 commented May 25, 2016

@jenssegers That's actually a good idea, I'll work on that instead.

@earnaway
Copy link

Any update on this PR or your code changes @GiampaoloFalqui?
Thanks.

@nickfan nickfan mentioned this pull request Jun 25, 2016
@nickfan
Copy link

nickfan commented Jun 25, 2016

since it's been a while,i created my PR #60 for this as @jenssegers asked.

@earnaway
Copy link

Thanks for the update @nickfan - I do notice that all the checks failed on that PR in StyleCI and Travis CI - @jenssegers might not be able to merge that in.

@nickfan
Copy link

nickfan commented Jun 27, 2016

@earnaway i have rewrite the code #60 for the check , it passed StyleCI and travis-ci,
but coverage/coveralls Coverage decreased (-9.9%) to 65.116% ,is that ok for merge?

@earnaway
Copy link

I guess we'll have to wait for @jenssegers to look over it and hopefully merge that into master.

@bkuhl
Copy link
Contributor

bkuhl commented Jul 31, 2016

Any updates on this? It'd be really nice to have Lumen support as there aren't any other packages that do this ATM.

@GiamPy5
Copy link
Author

GiamPy5 commented Aug 1, 2016

I am very sorry about the huge await, I have improved the Pull Request by using Inheritance. @jenssegers feel free to give a look.

@bkuhl
Copy link
Contributor

bkuhl commented Aug 1, 2016

Looks like we're unable to merge due to unresolved conflicts, can you look at those?

@GiamPy5
Copy link
Author

GiamPy5 commented Aug 1, 2016

@bkuhl Done!

@GiamPy5
Copy link
Author

GiamPy5 commented Aug 25, 2016

Any news?

@bkuhl
Copy link
Contributor

bkuhl commented Sep 20, 2016

The build is still failing

@jasonroman
Copy link

Has anyone taken a look at this in awhile?

@GiamPy5
Copy link
Author

GiamPy5 commented May 15, 2017

Build fixed and code updated with the latest master changes!

@RudyJessop
Copy link

Lumen support is desperately needed. Looking at the committed files I don't see a completed test case for src/RollbarLumenServiceProvider.php. Thats why coveralls coverage is decreasing/failing

@GiamPy5
Copy link
Author

GiamPy5 commented May 31, 2017

There isn't a way to test the LumenServiceProvider, it only exists for Laravel unfortunately.

@jelhan
Copy link

jelhan commented Jun 27, 2017

@jenssegers Is there any chance this PR is merged even if RollbarLumenServiceProvider is not covered by tests? If not: Would you accept a dirty work-a-round to get it tested or is this one blocked by orchestra/testbench not supporting lumen? Please note that crynobone said that adding lumen support to orchestra/testbench might be possible for lumen >= 5.2 but as far as I know nobody is working on it yet.

@wjgilmore
Copy link

FWIW this morning I followed this tutorial to create a custom service provider in order to use this package with a Lumen 5.2 project, and it worked like a charm:

https://recursionrecursion.co.uk/post/integrating-rollbar-with-lumen

@khoivm
Copy link

khoivm commented May 23, 2018

Hi all, any news for this? I know we can do the custom service provider, but it is best to have it on the package.

@chukkynze
Copy link

@wjgilmore I tried that tutorial for Lumen 5.7 and it uses deprecated methods and of course fails. If James Elliot would update it for Lumen 5.7 it could make lives easier but for now, it doesn't work for the current version of Lumen

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.