search

symfony-cmf / Routing

Routing component building on the Symfony Routing component
Other
288 stars 69 forks source link

Acrobat symfony5 support 2 #250

Closed dbu closed 4 years ago

dbu commented 4 years ago
Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Fixed tickets fix #248, #245
License MIT
Doc PR TODO

Continuation of the work by @acrobat

I realized that the VersatileRouterInterface is used to provide meaningful debug messages when something did not work, and i think for that its worth to keep it. So i only deprecated the supports method. And adjusted the debug message method to handle the new mode as well.

dbu commented 4 years ago

@acrobat wdyt ? only problem i have is that i get mismatches on the @expectedDeprecation with a %A in the expectation. do you have an idea what happens here? if not, i tend to just drop those expectations, we do see the depreactions currently work and they will be removed in the next major anyways...

dbu commented 4 years ago

@alexander-schranz can you try this branch in sulu and report if you run into any issues? are you using the bundle as well, or only the component? the bundle likely also needs some adjustments.

acrobat commented 4 years ago

@dbu Looks good! If you apply this diff, the expectedDeprecations work again (there was a (backtick). missing at the end! The %A is indeed confusing but it has nothing to do with the error itself, dunno why it's displayed 🤷‍♂️

Diff: https://gist.github.com/acrobat/c929ad46a84a91d1f51f18755e6dd85c

alexander-schranz commented 4 years ago

@dbu looks good from the sulu side. Did not get any error and the routing did work. We are also using the Bundle I used the symfony-5 branch there to test it.

acrobat commented 4 years ago

I think the travis build got "lost" during the maintenance this morning. Can you restart a new build in the travis interface?

dbu commented 4 years ago

triggered a manual build: https://travis-ci.org/github/symfony-cmf/Routing/builds/677716799 (that will not update the PR here, but if its green, i guess i will merge)

do you have any inputs on the code @acrobat or does it all look good?

dbu commented 4 years ago

oh, actually travis-ci is very clever and did update us here. and even better, its green

acrobat commented 4 years ago

triggered a manual build: https://travis-ci.org/github/symfony-cmf/Routing/builds/677716799 (that will not update the PR here, but if its green, i guess i will merge)

do you have any inputs on the code @acrobat or does it all look good?

The code looks good to me! 🚢