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

laravel/framework as a dependency #49

Closed
cdarken opened this issue Dec 20, 2016 · 8 comments
Closed

laravel/framework as a dependency #49

cdarken opened this issue Dec 20, 2016 · 8 comments
Milestone

Comments

@cdarken
Copy link

cdarken commented Dec 20, 2016

I'm using this package with Lumen. But because laravel/framework is specified as a dependency, it pulls the entire framework on composer install. I was wondering, maybe you can you limit the dependencies to separate Illuminate packages.

@potsky
Copy link
Owner

potsky commented Dec 20, 2016

Hello,
indeed, I use this dependency because of the way of Laravel (according to version) to boot service providers and declare facades.
Do you have an idea of the mutual composer dependency between Laravel and Lumen ?

@potsky
Copy link
Owner

potsky commented Dec 20, 2016

  • lumen-framework embeds the Laravel\Lumen\Application class
  • laravel-framework embeds the Laravel\Foundation\Application class

It seems to be a problem... Do you know a package which has the expected behaviour and which works with both Laravel and Lumen ?

@cdarken
Copy link
Author

cdarken commented Dec 20, 2016

Looking into it.

@cdarken
Copy link
Author

cdarken commented Dec 20, 2016

There are 2 options that I see right now:

  1. make a new service provider for Lumen (like https://github.com/nordsoftware/lumen-cors/blob/develop/src/CorsServiceProvider.php)
  2. recommend the usage of the package https://github.com/irazasyed/larasupport that adds some helper functions that you use in the package
    Both require minimal changes outside of the service provider, like usage of config() helper instead of Config::get which would require creating an alias for the Config facade in the bootstrap/app.php of the application using the package.
    I can create a pull request for you to take a look if it's ok.

@cdarken
Copy link
Author

cdarken commented Jan 5, 2017

Hi and happy new year. Did you think about this? I've pushed a branch with the modifications for Lumen compatibility on my fork.

@potsky
Copy link
Owner

potsky commented Jan 6, 2017

Hi !

Happy new year to you too!

Your commit is here : cdarken@0e7dbd5

  • I can simply change the composer dependency and use yours given that the Laravel Framework has this dependency too with the same version logic.
  • I can use the same config helper given that it exists in Laravel

Than I can check the used framework and performing a if / else on these items :

  • Config::has
  • app('path')

I don't want to manage a new branch especially for Lumen ! I lose time with branches. When I add features, I code in master and then merge modifications manually on each branch...


I have added this in the bootstrap/app.php file :

$app->register( Potsky\LaravelLocalizationHelpers\LaravelLocalizationHelpersServiceProvider::class );

But it seems there is no configuration file in Lumen... Just a question : how do you load my package in lumen and manage package configuration ?

@cdarken
Copy link
Author

cdarken commented Jan 6, 2017

For loading a config file you copy it manually in config/ and you use this call in bootstrap/app.php:

$app->configure('laravel-localization-helpers');

@potsky
Copy link
Owner

potsky commented Jan 6, 2017

Ok... So I will finally create a new branch especially for Lumen with the according documentation.

potsky added a commit that referenced this issue Mar 1, 2017
@potsky potsky closed this as completed in 1ccd661 Mar 1, 2017
@potsky potsky modified the milestone: 2.x.5 Mar 16, 2017
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

No branches or pull requests

2 participants