Added Mapper pass and expanded config #29
Conversation
- Added Mapper compiler pass to register additional Mappers with the Dynamic router - Expanded the configuration to accept "route_repository_class" and "content_repository_class"
|
indeed, sounds like we miss something here. i will review tomorrow in detail. i was thinking for a while now to rename the mapper into RouteEnhancer that can add/change anything in the route $defaults array, not just the _controller. the Routing component is not tied to Symfony and thus _controller should have no specific meaning to it. if you have an opinion on this, i would love feedback on symfony-cmf/Routing#20 - but this should not block this pull request, we could still refactor once we change to generic enhancers. |
|
@lsmith77 does this conflict with symfony-cmf/Routing#28 ? or do we just add an additional parameter in Routing 28 to specify the subpath? |
|
need to look at this some more but you add attribites to tags |
| @@ -87,6 +87,15 @@ private function setupDynamicRouter(array $config, ContainerBuilder $container, | |||
| $loader->load('cmf_routing.xml'); | |||
| $container->setParameter($this->getAlias() . '.routing_repositoryroot', $config['routing_repositoryroot']); | |||
|
|
|||
| if (null !== $config['route_repository_class']) { | |||
| $container->setParameter('symfony_cmf_routing_extra.route_repository_class', $config['route_repository_class']); | |||
There was a problem hiding this comment.
@lsmith77 do you know if this is the right way to do things? simply setting this value in the app/config/config.yml parameters: section would result in the same. do we want to explicitly expose some but not all classes as configuration?
There was a problem hiding this comment.
no. it's always better to explicitly support this. makes it clear that it's intended to be configurable
|
@lsmith77 do you want to wait merging until you found time to look into it, or can we merge and refactor later if needed? |
|
the travis fail seems to come from master, not exactly sure what is going on but probably not related to the PR |
|
if you think it works merge. I will be back on Wednesday |
Added Mapper pass and expanded config
|
thanks @dantleech - i will go and refactor the naming and doc soon so that its clear this is not just for controllers but any kind of defaults. it will be a BC break on the tag name used here. and on the interface name as well. but first i want to fix the tests, its not fun to refactor when half the tests don't work. |
Hi,
I couldn't find a way to register a new mapper, so have added a mapper compiler pass. I also added configuration options for the route and content repository classes.