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
Consolidate plugin files #34
Conversation
<directory name="tests" /> | ||
</ignoreFiles> | ||
</projectFiles> | ||
|
||
<issueHandlers> | ||
<LessSpecificReturnType errorLevel="info" /> | ||
</issueHandlers> | ||
|
||
<stubs> | ||
<file name="src/Stubs/LumenApplication.stubphp"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to add a stub for the lumen application file, instead of requiring the lumen framework as a dependency. I assume most people using this package will be using laravel and not lumen, so I think this is acceptable
$app = (new static)->createApplication(); | ||
} | ||
|
||
if ($app instanceof \Illuminate\Contracts\Foundation\Application) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find having these two conditional statements here much more readable than splitting the logic via inheritance
$app->boot(); | ||
} | ||
|
||
$app->register(\Barryvdh\LaravelIdeHelper\IdeHelperServiceProvider::class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be safe to call register
on all the app instances. Register exists on both the lumen application as well as the laravel application. Both seem to short circuit if getProvider
is true, so we don't need to explicitly call that ourselves.
Mind rebasing this? |
66bf394
to
c15f1f7
Compare
My main goal of this was to:
Everything here should be functionally the same, it is just a refactor.