-
Notifications
You must be signed in to change notification settings - Fork 171
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
Refactor Laravel-specific code #320
base: master
Are you sure you want to change the base?
Conversation
Thanks, did you get a chance to test this? Are the changes required or can we bump the versions for now? |
I've tagged a versions which allows Laravel 10, but will consider this as an improvement. |
61a8900
to
37a77d8
Compare
@barryvdh thanks for looking into this! I wasn't able to test this in my application yet, but a quick test round in a fresh Laravel skeleton allowed me to publish everything and use basic operations without any problems. So I think the Laravel 10 support is good to go! I've rebased my branch so you can still merge in the changes. You're correct none of them are actually required for Laravel 10, they basically were the easiest way for me to check basic compatibility in terms of classes/functions using just an IDE. Nonetheless I think they're very useful to keep this package up to date ;) |
Subtle reminder this is still open; if there is anything I can do here, let me know! |
Also refactor some cases where $this->app could be used and use constructor property promotion
Also sort dependencies for better readability
37a77d8
to
6e5bba0
Compare
This PR aimed to add support for Laravel 10, which was later introduced in #321.
It's remaining parts is refactoring some things to allow for easier static code analysis within IDE, to allow spotting deprecations and/or removals from Laravel more easily. It also specifically lists all used Illuminate dependencies in
composer.json
.ToDo: