search

metaclass-nl / filter-bundle

Filter bundle for API Platform, Filter Logic
MIT License
50 stars 9 forks source link

feat: allow symfony6, add missing doctrine/orm in composer.json #3

Closed art-cg closed 2 years ago

art-cg commented 2 years ago

Hi,

this is not yet tested. Due to missing test-config in this bundle. I am working on that in another PR.

I added "doctrine/orm" as dependency. But i am not sure about the minimum version. Would be great if you can have a look at this PR.

Greetz

metaclass-nl commented 2 years ago

Thanks a lot, sounds very nice, i'll take a quick look friday, but it may take more time before i can really look into it. I am currently working on something else and need to finisch that before i can really spend time on this.

metaclass-nl commented 2 years ago

Thanks!

I did not add a doctrine/orm dependency because the composer.json of api-platform already does. I do not want maintain all version numbers of all inderect dependencies. If api platform is maintained correctly according to semantic versioning rules there should not be any breaking of backward compatibility unless a major version number is released. I thought with the config 

"api-platform/core": "^2.5"

composer will not install any new major versions of api platform so erverything should keep working. But maybe i am wrong and should there be a config for every dependency whose methods are called directly by the code in the bundle. Then it should be

"doctrine/orm": "^2.8",

Because that is in the config of the app within which i tested the bundle.

I have seen the api-platform/core issue about Symfony 6 but i figured it would be part of the next major version, but now i see v2.6.7 requires 

"symfony/http-kernel": "^4.4 || ^5.1 || ^6.0",

But if i should include a config for every dependency whose methods are called directly by the code in the bundle, i can not merge this until i tested it with Symfony 6.0. That will take some time.

art-cg commented 2 years ago

Thanks for taking a look inside this PR. "api-platform/core" does not require "doctrine/orm": see https://packagist.org/packages/api-platform/core But beside of that, it is considered best practice to require all libs that are required for the basic code to work. (optional things in "suggests"). It is easier to test and work on the bundle. I changed it to "doctrine/orm": "^2.8" It is your bundle, so you habe the right to decide.

Maybe my other (unfinished) PR helps with testing.