search

symfony-cmf / Routing

Routing component building on the Symfony Routing component
Other
289 stars 70 forks source link

[WIP] Add support for symfony 5 #SymfonyHackday #240

Closed vincentmoulene closed 4 years ago

vincentmoulene commented 4 years ago
Q A
Branch? "master"
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc PR

SymfonyHackday

Pull request made for the support of Symfony 5. I am a beginner in open source, so let me know if everything is good.

I saw the tests failed with another component symfony-cmf/testing but I don't know how replace it in the composer.json file to test with another branch. Let me know and I will update this PR.

Thanks

dbu commented 4 years ago

depends on https://github.com/symfony-cmf/Testing/pull/194

dbu commented 4 years ago

thank you for this pull request! we need to wait for the testing component to be updated to allow installation together with symfony 5.

dbu commented 4 years ago

i did some fixes for the build in master. could you please rebase on master (and solve the conflicts) to see if things now work? it is quite likely that we need to do something about #238...

acrobat commented 4 years ago

@dbu I would like to help with the sf 5 support. But how would you like to solve the UrlGeneratorInterface parameter issue, without breaking BC and still support sf 4 and 5 in the current major?

dbu commented 4 years ago

hi @acrobat , thanks for the offer! yeah that parameter typing is indeed a major issue. afaik in the past we abused the weak type checking to do something that symfony had not intended.

i think the only way forward is to place the route object in the $attributes and use "" as route name or something like that.

our router implementation will need some sort of version check to determine which method signature to use, with some PHP hackery like this: https://github.com/kuraobi/DoctrinePHPCRBundle/blob/acc7abfd5e3ea865ef77ef110f41232e28c58cc7/src/DependencyInjection/DoctrinePHPCRExtension.php#L572-L595

the old signature would take the route object from the $route parameter, but emit a deprecation warning, saying you should pass the route as attribute.

we need to adjust our code and templates to pass the route as attribute, as well as adjust the documentation.

would be great if you take a go at that. i pushed support-for-symfony-5 to this repository so that you can do pull requests against that branch.

dbu commented 4 years ago

finished in #250