[Routing] Remove usage of deprecated _scheme requirement#9585
[Routing] Remove usage of deprecated _scheme requirement#9585fabpot merged 0 commit intosymfony:2.3from danez:2.3
Conversation
There was a problem hiding this comment.
As the only requirement checked inside doGenerate was '_scheme' I exchanged $requirements with $requiredSchemes. So the semantic of this argument changed. But as this method is protected and not a public api, I thought it may be okay. Do you think this is a BC? Otherwise the only solution would be adding an additional optional parameter.
There was a problem hiding this comment.
It is a bc break and cannot be merged as this in 2.3.
The method is called by several other libraries that reuse the routing component.
There was a problem hiding this comment.
We cannot make such a BC break in 2.3, but not even in master.
|
Okay I removed the BC on the doGenerate-method and added the schemes-requirement as new optional argument. In this case if this method would be called as before this PR it would still do the same. |
There was a problem hiding this comment.
Please use: assertTrue() & assertFalse().
|
By accident I completely destroyed the commit in my fork, and it seems nothing was merged. I now had to restore the commit from the reflog and gonna open a new PR. |
…anez) This PR was merged into the 2.3 branch. Discussion ---------- [Routing] Remove usage of deprecated _scheme requirement **This is exact the same commit as it was in #9585, which was not merged due to my fault. Sorry for the noise.** | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #8898, #8176 | License | MIT | Doc PR | I removed all usages of the deprecated _scheme requirement inside the Routing Component. Most parts were pretty easy and after multiple refactorings I came up with the solution to have a Route::hasScheme() method and check against this method. I also checked for performance and after trying in_array, arra_flip+isset and foreach, the last one was clearly the winner. https://gist.github.com/Danez/7609898#file-test_performance-php I also adjusted all tests that test '_scheme' to also check the new schemes-requirement. Commits ------- 557dfaa Remove usage of deprecated _scheme in Routing Component
I removed all usages of the deprecated _scheme requirement inside the Routing Component.
Most parts were pretty easy and after multiple refactorings I came up with the solution to have a Route::hasScheme() method and check against this method.
I also checked for performance and after trying in_array, arra_flip+isset and foreach, the last one was clearly the winner.
https://gist.github.com/Danez/7609898#file-test_performance-php
I also adjusted all tests that test '_scheme' to also check the new schemes-requirement.