search

symfony-cmf / routing-bundle

Symfony bundle to provide the CMF chain router to handle multiple routers, and the dynamic router to load routes from a database or other sources.
160 stars 78 forks source link

Add return types for sf6 #472

Closed trsteel88 closed 2 years ago

trsteel88 commented 2 years ago

Added required return types for symfony 6.

getContentId() was also updated to return null (instead of void). The phpdoc indicated this should be string|null

dbu commented 2 years ago

thanks a lot! this makes me realize that there is no way around the BC break and doing a new major version for symfony 6. our Route model extends the symfony route object, and that model is supposed to be an extension point to implement your own stored routes...

could you make this a major version bump (adjust the branch-alias in composer.json and update the CHANGELOG accordingly?). and drop PHP 7.x support, have all builds use PHP 8.0 or 8.1 so that static works. if we have to drop older symfony versions for that to work, lets drop those as well.

i would still prefer to have just the steps necesary to work with symfony 6 in this PR, and then rebase #471 to have that one add strict typing and return type declarations to our own code, for a clearer git history.

trsteel88 commented 2 years ago

@dbu, update composer.json and the test workflow. However, I think a new version of the routing component will need to be released first (as it does not support symfony 6)

trsteel88 commented 2 years ago

@dbu, it looks like just the prefer lowest test is failing. I think because some Symfony 5.4 packages are being installed. Should this be scrapped from the matrix? Or, how do enforce those packages to use >=6.0?

dbu commented 2 years ago

the lowest version build proved its worth here. it is meant to prevent insallation of this library with incompatible components. i tried some things and found that a too old doctrine/common does not handle the : static return type declaration correctly on proxies. so i had to adjust the constraint and now the lowest build is fine too.

dbu commented 2 years ago

merged in #473, thanks a lot!

trsteel88 commented 2 years ago

No worries @dbu. In case you missed it, the testing bundle also needs a bump. I used dev-master as 4.2.0.

dbu commented 2 years ago

i am aware. currently trying to do the static typing on the routing library so that we then can release it, and then will see about releasing testing and then finally the bundle.