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

Lumen compatability #247

Merged
merged 2 commits into from Aug 20, 2019

Conversation

@andrewnclark
Copy link
Contributor

commented Aug 16, 2019

Q A
Bug fix? No
New feature? No
Fixed tickets #235

Following the conversion in #235 this PR updates the Laravel service provider to detect when it is being used in a Lumen application and avoid calling $this->publishes as the publish command is not available.

We could potentially make a separate Lumen service provider if preferred.

With the above changes in mind, I also updated the documentation to provide steps for install phpinsights in a Lumen app.

@andrewnclark andrewnclark force-pushed the andrewnclark:lumen-compatability branch from b184a29 to fff245e Aug 16, 2019

$this->publishes([
__DIR__ . '/../../../../stubs/laravel.php' => $this->app->configPath('insights.php'),
], 'config');
if (strpos($this->app->version(), 'Lumen') !== 0) {

This comment has been minimized.

Copy link
@nunomaduro

nunomaduro Aug 16, 2019

Owner

Did you double check that this code is working as expected in a normal lararel application?

This comment has been minimized.

Copy link
@olivernybroe

olivernybroe Aug 16, 2019

Collaborator

There is a method called version in Laravel, and that one just returns a version number in Laravel.

However I think a "better" way to do this is to check $this->app implements Illuminate\Contracts\Foundation\Application, because then we know it's a Laravel project.

This comment has been minimized.

Copy link
@andrewnclark

andrewnclark Aug 17, 2019

Author Contributor

Ah I did write the instanceof version first and changed it, I'll switch back to... always trust your initial thoughts!

docs/get-started.md Outdated Show resolved Hide resolved

@andrewnclark andrewnclark force-pushed the andrewnclark:lumen-compatability branch from 4f20c54 to 2f3cfee Aug 19, 2019

@andrewnclark andrewnclark force-pushed the andrewnclark:lumen-compatability branch from 2f3cfee to 842ea54 Aug 19, 2019

@andrewnclark

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2019

Made the requested changes and rebased onto the current version of master. I can also confirm that I have tested it with a Laravel installation and the service provider still works as expected.

@nunomaduro
Copy link
Owner

left a comment

Perfect.

@nunomaduro nunomaduro merged commit a0a8c61 into nunomaduro:master Aug 20, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.