-
-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TASK: Use RouterInterface instead of concrete implementation #1228
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
@@ -37,7 +37,7 @@ class RoutingCommandController extends CommandController | |||
|
|||
/** | |||
* @Flow\Inject | |||
* @var Router | |||
* @var RouterInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RoutingCommandController invokes methods on the router that are not part of the interface, namely getRoutes()
@@ -49,7 +49,7 @@ class InternalRequestEngine implements RequestEngineInterface | |||
|
|||
/** | |||
* @Flow\Inject(lazy = false) | |||
* @var Router | |||
* @var RouterInterface | |||
*/ | |||
protected $router; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't adjust the return type hint of the respective getter (https://github.com/Torsten85/flow-development-collection/blob/39e601e0312fe5bc3a8ef0e9f4a937063e3cda4d/Neos.Flow/Classes/Http/Client/InternalRequestEngine.php#L138)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for that, was a bit in a hurry ;)
@@ -292,6 +292,9 @@ Neos\Flow\Property\PropertyMapper: | |||
# ResourceManagement # | |||
# # | |||
|
|||
Neos\Flow\Mvc\Routing\RouterInterface: | |||
className: Neos\Flow\Mvc\Routing\Router | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You changed this in the section "ResourceManagement", There is already one for "MVC"
Makes sense in general, but there are some minor issues with the current change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good now (by reading)
TASK: Use RouterInterface instead of concrete implementation
Currently the concrete
Router
Implementation is used instead of the more flexibleRouterInterface
.This PR ensures the
RouterInterface
is used.