[Routing] clean up of RouteCollection API #6022
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… options without adding a prefix at all.
so RouteCollection::addPrefix('', array('default' => 'value')) was used in addCollection() to add defaults to child routes, but not adding a prefix. this was just a hack. instead RouteCollection now offers a dedicated method to add such configs.
it has been deprecated because adding options has nothing to do with adding a path prefix
these prefix, defaults, requirements params were redundant to the behavior of addPrefix. and with the addition of hostnamePattern, this method did basically everything although there are already specific methods for these use-cases. the single responsibility of the methods provides a better API
… than array anyway
…nt with addPrefix which also accepts placeholders with defaults and requirements
it also fixes the xml and yaml loader that was not using the empty hostname pattern when importing a resource to reset the hostname pattern for it.
|
@fabpot this is finished and rebased. I switched from |
|
|
||
| $matcher = new UrlMatcher($coll, new RequestContext()); | ||
| $this->assertEquals(array('_route' => 'bar'), $matcher->match('/new')); | ||
| } | ||
| public function testWithHostname() |
| Before: `$parentCollection->addCollection($collection, '/prefix', array(...), array(...))` | ||
| After: | ||
| ``` | ||
| $collection->addPrefix('/prefix', array(...), array(...)); |
There was a problem hiding this comment.
What about using the new methods instead of the deprecated one here (where you pass additional arguments to addCollection)?
There was a problem hiding this comment.
I don't understand. addPrefix('/prefix', array(...), array(...)); with defaults and requirements is not deprecated.
fabpot
added a commit
that referenced
this pull request
Dec 11, 2012
This PR was merged into the master branch. Commits ------- 237bbd0 fixed and refactored YamlFileLoader in the same sense as the XmlFileLoader 392d785 removed covers annotation from loader tests and unneeded use statements 45987eb added tests for the previous XmlFileLoader fixes b20d6a7 ensure id, pattern and resource are specified 8361b5a refactor the XMlFileLoader 83fc5ff fix namespace handling in xml loader; it could not handle prefixes 1a60a3c make resource and key attributes required in xsd 02e01b9 improve exception messages in xml loader 51fbffe remove unneeded cast 358e9c4 fix some phpdoc Discussion ---------- [Routing] improve loaders BC break: no Main points: - fixed Xml loader that could not handle namespace prefixes but is valid xml - fixed Yaml loader where some nonsesense configs were not validated correctly like an imported resource with a pattern key. Added tests for all. Some refactoring + a few tweaks like better exception messages and more consistency between Xml loader and yaml loader. See also commits. --------------------------------------------------------------------------- by Tobion at 2012-12-07T18:16:08Z @fabpot this is ready --------------------------------------------------------------------------- by Tobion at 2012-12-11T17:30:10Z @fabpot rebased. Please don't squash to one big commit where one does not see what changed why. --------------------------------------------------------------------------- by fabpot at 2012-12-11T17:32:40Z I only squash when most commits are CS fixes and feedback. --------------------------------------------------------------------------- by Tobion at 2012-12-11T17:37:49Z Well, you squashed #6022 so it's not possible to revert a specific deprecation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BC break: only the internal behavior of addPrefix()
Deprecations:
and it's also useless since we dont have a tree anymore)
Reasoning see commits and changelog.