Skip to content
This repository has been archived by the owner on Jan 11, 2022. It is now read-only.

Added Lumen support and config variables are set via the .env #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ibexcore
Copy link

@ibexcore ibexcore commented Jun 6, 2018

Added Lumen support. Should work on Laravel as well.
Set most of the config to be configurable via .env configuration basically removing the need to publish the config.

@ibexcore
Copy link
Author

ibexcore commented Jun 6, 2018

Annoyingly, the tests pass on my local Laravel version but all fail in my Lumen version due to the Orchestra dependency.
If you have any ideas why this is happening, I'd be happy to fix it. Otherwise, I'll attempt to fix the issues over the weekend.

@barryvanveen
Copy link
Contributor

Hi @ibexcore,

Thanks for the PR! It looks like a good contribution but we need to fix some issues before this can be merged.

  1. The most obvious problem is that all tests fail. See https://travis-ci.org/swisnl/laravel-graylog2/jobs/388779084 for more information. It looks like you call pushHandler() on app('log') while you should call it on app('log')->getMonolog()?
  2. There has been a change to the service provider that causes a conflict. Could you merge the changes from master into your PR?
  3. Lastly the diff contains a lot of empty changes, probably due to changes in file endings. Could you clean this up so we have a cleaner commit?

Thanks again for your work.

Barry

@barryvanveen
Copy link
Contributor

@ibexcore any update on this? Any thoughts on my comments?

@amanjain1992
Copy link

Hi @ibexcore / @barryvanveen ,

Any update on this since i am looking forward to integrate this with lumen and even i am facing same problem

@barryvanveen
Copy link
Contributor

Hi @amanjain1992

I haven't heard from @ibexcore after his initial PR. I have added some comments that need to be addressed before this feature can be merged. Since I don't need Lumen support (and don't have a Lumen project) myself, I will not do this myself.

You are welcome to help implementing support if you can. If the contribution is of sufficient quality I will be happy to merge it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants