Bump dependencies to allow for Laravel 9 #205
Conversation
|
It would be great to get this merged! |
|
yep trying to update to 9.0 and this is breaking stuff :D |
|
@jdanino Could you update the README.md to show support for Laravel 9? May help to get the PR merged. Thanks for updating composer.json. |
done |
|
there is a big possibility there is no one looking at this, not even the issues are being reviewed for like 1 year, does anyone know anyone in amazon :D |
|
Hi there, I'm from amazon. Gonna talk to the team about it during a meeting tomorrow and try to get this on the docket soon. If you'd like to reach us, I try to respond on gitter as quickly as I can, and tagging me helps me to see it sooner. |
|
Would love to see this in asap!! this is a blocker for us upgrading to Laravel 9 |
|
please, accept the merge request asap! I can`t upgrade my project for laravel 9 |
|
This PR was also asking for the same thing; unfortunately this is a breaking change and we need to bump our minimum and maximum supported versions, which will take some time. We are discussing it now and working on making a final decision and then shortly thereafter will make an official public announcement if we do. |
|
@SamRemis Why do you need to give customers on those older versions time to upgrade? |
@SamRemis - I, too, am curious about this. Why not tag a new version for this release now and drop support for old versions later? Getting an important release out shouldn't be dependent on when you can drop support for old versions. This is very important to us, and I would love to see this merged asap. |
|
@SamRemis - Very important for me and my guys as well, why can you not release for 9 and let others upgrade as and when they need/want...? Please merge this... |
|
I may be able to merge this one sooner, but I have to at least get to the point of officially announcing the deprecation campaign. This is one of my highest priorities since it's clearly important to the community, I'll merge this as soon as I can confirm no breaking changes. It may be a fair bit of work to write and test something like this with a new version. In that case, it's not just pressing a merge button. If I were to merge this, it would likely break code. This needs unit test refactoring, versioning changes, function changes, and potentially a major version change. |
None of that makes any sense. This is a trivial change. The change here is just saying that it works with Laravel 9. It does. |
Yeah I don't get it either. It's impossible to introduce a backward compatibility issue by merging this PR because all this is doing is saying "hi I work with Laravel 9", which is true with zero other code changes. |
|
I deleted a comment tagging someone who no longer works for amazon |
|
I have been a passionate user of Amazon Web Service and PHP development for many years. Unfortunately, the combination of AWS and PHP is a problem that has always been there and seems unsolvable. Amazon doesn't like PHP... It's True.. dump( $amazon->likephp() ) = false |
Could've spent that 3 seconds hitting the merge button and still had a second to spare |
|
@SamRemis I appreciate that you are taking this seriously, and I'm sorry to push like this, but you're actually creating an unnecessary headache for AWS customers in this case. You do realize this is the Laravel package and not the PHP SDK itself, right? If so, can you please explain why you believe changing the composer requirements to allow Laravel 9 impacts any past version of Laravel? We are literally talking about four files here: a facade, a service provider, and some config stuff that get overwritten anyway. Comments removed we are looking at less than 100 lines of code. Nothing in Laravel 9 has changed that would lead to any code changes. |
|
@jonnylink thanks for understanding and patience; looks like my initial assessment of the significance was probably incorrect. I still need time to do my due diligence. This will be my first priority and I'll work on it as soon as I'm able, which is early next week. If what I've read so far from the docs is a good indicator, it shouldn't take long at all once I've started. |
|
Thank you @SamRemis for helping us with moving forward on this. As this is a library used by many, proper checks should be done and we understand it. If it helps, we forked the project and made same changes as in this PR (basically allowing use of Laravel 9). We already use it in production on a couple of core projects. No issues found so far. |
|
Thanks for making a second assessment, @SamRemis. When you get down to it, this package is really just a service provider for the SDK. As you can see in the upgrade guide there aren't any changes to the service providers, so there truly isn't anything to do. The unit tests as written should adequately cover things. Looking forward to this getting pushed out next week. |
|
MERGE THIS! |
|
I know this is annoying and inconvenient for some of us, but please be kind. You aren't without temporary or even permanent alternate solutions (like forking, creating your own service provider, etc.) if upgrading to Laravel 9 is that important to you. |
|
Counter point: It seems pretty clear that this package (and AWS customers) aren't going to get the support at a professional level. This PR literally just adds EDIT: that said I'm also in agreement we don't need to be rude. |
|
@jonnylink Just because the aws package exists doesn't stop you from publishing your own on packagist if you feel that way.
|
|
@hailwood off-topic. The PR is good and should have been merged a month ago. |
|
Waiting all this time for such a simple change is truly incredible. The answers of some of those responsible are ridiculous and do not deserve a debate. As for me I can also wait, I will move the update of my AWS customers to next month, but in the meantime I have presented to AWS Support as a premium user with 35 midrange customers my due reflections. |
|
Resolved by #209 |
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.