-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fix 139 #142
Conversation
private function handleRoute($routeInfo, PennyEventInterface $event) | ||
{ | ||
private function handleRoute( | ||
RouteInfoInterface $routeInfo, |
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.
remove RouteInfoInterface type hint
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.
Why?
@@ -133,13 +133,15 @@ public function run($request = null, $response = null) | |||
/** | |||
* Handle Route. | |||
* | |||
* @param mixed $routeInfo | |||
* @param PennyEventInterface $event | |||
* @param RouteInfoInterface $routeInfo |
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.
@param mixed $routeInfo
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.
Why? The dispatch method triggers exceptions or return RouteInfo object
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.
but the exception won't be executed, as the type hint is RouteInfoInterface
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.
this is the internal method that handle RouteInfoInterface check, so param need accept not a RouteInfoInterface, so the RuntimeException will be executable!
I will re-create PR for it. |
@gianarb you're wrong on rebase |
Rebase #139