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

Add support to Laravel 9 #129

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

Conversation

gallib
Copy link

@gallib gallib commented Jan 24, 2022

Hi,

this PR just add support to upcoming Laravel 9.0

@felixkiss
Copy link
Owner

Sorry, I missed this completely!

Will verify soon ™️ 🙃

@robbielove
Copy link

closes #131

@robbielove
Copy link

robbielove commented Mar 6, 2022

@felixkiss I would really like this to be merged due to reasons explained in this PR for a different package where I had the same problem: laravelista/comments#183 (comment)

I have looked through the codebase vs the upgrade guide vs this PR
The changes would appear to work, but I haven't specifically tested this functionality (I have checked that I can update by composer), or even know how to. I am just a downstream user of this package through rinvex/laravel-support.

I am being held up by the reasons mentioned previously - but in short, If you merge this to master or another branch then that branch can be used without having to add a custom repo fork (laravel shift fork, etc) to each of my apps (just because they use a package which in turn uses this package).

Looking at your branch structure - there's only a single master branch - I would say your branching strategy appears different to that used in laravelista/comments (you probably don't use master as an experimental or unstable branch) - therefore I would suggest you simply change the base of the shift gallib PR to be a new branch called l9-compatibility and merge the shift gallib PR into that - then we can use and test the l9 changes without affecting master branch stability which I suspect you want to preserve, and anyone who wants to use/test the l9 can use it without specifying the laravel shift custom repo. When it's tested enough we can simply PR or merge directly to master - eg. the best of both worlds or win/win.

This is the last remaining dependency for all of my laravel apps to be upgraded - I would appreciate it if you could consider my requests. Any help is appreciated and I thank you for your contribution to this package which in turn helps packages that my package uses. ❤️

edit: my mistake; the laravelista PR was a shift PR, thanks @gallib for this PR

@singleseeker
Copy link

Any update?

@mattvb91
Copy link

mattvb91 commented May 2, 2022

@felixkiss any chance of getting this in?

@mikkoaf
Copy link

mikkoaf commented Jun 20, 2022

Any update on this PR?

@EM-LilianaIturribarria
Copy link

Hi,
any update on this?
we need support for laravel 9 and php > 8.1

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

Successfully merging this pull request may close these issues.

7 participants