flugg / laravel-responder

A Laravel Fractal package for building API responses, giving you the power of Fractal with Laravel's elegancy.
MIT License
861 stars 86 forks source link

Add support for Laravel 7 and PHPUnit 9 #158

Closed fixmk closed 4 years ago

fixmk commented 4 years ago

https://github.com/laravel/laravel/pull/5157/files#diff-646a4842abf9023c975a9a1658a68b0aR47 and https://github.com/sebastianbergmann/phpunit/issues/3494 change does not allow to get us to the 7th version. Laravel 7 has introduced his own ArraySubset class.

Advise to isolate these changes under new major version, because we cannot support v6.18.0 and v7.0 together with these changes.

These changes introduce minimal PHP version 7.3

flugg commented 4 years ago

Thanks for this! Looks like you've put quite some work into this. Will look into this over the weekend and get the package ready for v7. With that said, it's a bit unfortunate if we have to change the requirements this drastically, even though it's a new major version.

fixmk commented 4 years ago

There's not much changes, it was just declaration.

By the way, I tried to experiment more with back-compatibility between 6.18 and 7.0, but seem to be that their jump to Symphony 5 is the main problem. I tried to switch to PHPUnit 8.5 to make it working also. You can see the last closed PR in your project with my unsuccessful attempts 😁

We definitely should stop thinking about support of 7.0 and 7.1. Laravel 7 has a dependency on PHPUnit 8.5 which has a minimum support of 7.2.5

If it is not urgent, then we can think about making another PR with patch to make smooth transition to minimum requirement of Laravel 6.x with PHPUnit 7.x (and raising minimum requirements of PHP to 7.2) and then after to apply this one already as a major version.

In addition they changed the way how they will version next releases, so we should be ready for each half year update of composer. Or to do it in advance. Up to you!

flugg commented 4 years ago

Hi,

I've done some tests and it seems like everything works like expected if we keep the old dependencies as the only breaking changes are related to the tests.

While I'm a fan of moving forward with versions I would preferably like to wait until the next major version is released before changing them (which is happening very soon).

Unless you have some objections to this, perhaps you could revert back to the old dependencies, but add ^7.0 to the list like: "5.1.* || 5.2.* || 5.3.* || 5.4.* || 5.5.* || 5.6.* || 5.7.* || 5.8.* || ^6.0 || ^7.0", then this should be ready for merge and release. Thanks!

fixmk commented 4 years ago

In

Unless you have some objections to this, perhaps you could revert back to the old dependencies, but add ^7.0 to the list like: "5.1.* || 5.2.* || 5.3.* || 5.4.* || 5.5.* || 5.6.* || 5.7.* || 5.8.* || ^6.0 || ^7.0", then this should be ready for merge and release. Thanks!

What's about testbench and phpunit ? The dependencies suppose to be also downgraded ?

flugg commented 4 years ago

We can keep the newest dependencies for all dev dependencies like you added in this PR. We could've added a 7.0+ requirement for the Illuminate stuff in devDependencies, however, if you're working locally on the package the newest version will be installed anyway, so probably no need.