Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

marshalBadControllerEvent argument type hinting #161

Merged
merged 1 commit into from
Jun 30, 2016

Conversation

autowp
Copy link
Contributor

@autowp autowp commented Jun 17, 2016

Exception -> Throwable

At line 102 marshalBadControllerEvent used with Throwable argument type

Exception -> Throwable

At line 102 marshalBadControllerEvent used with Throwable argument type
@@ -216,7 +216,7 @@ protected function marshalBadControllerEvent(
$controllerName,
MvcEvent $event,
Application $application,
\Exception $exception
\Throwable $exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not gonna work on php 5.6 :(
please have a look in composer.json, and it still must work on: "php": "^5.6 || ^7.0"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webimpress It works fine on PHP 5.6; the class does not need to exist for a typehint check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed that this was in the method signature and not the docblock. I'll remove the typehint during merge.

@michalbundyra
Copy link
Member

it was already resolved in release 2.7.9:
https://github.com/zendframework/zend-mvc/pull/153/files

should be also ported to release v3

/cc @weierophinney

@weierophinney weierophinney added this to the 3.0.2 milestone Jun 30, 2016
@weierophinney weierophinney self-assigned this Jun 30, 2016
@weierophinney
Copy link
Member

Thanks @autowp !

@weierophinney weierophinney merged commit 800117e into zendframework:master Jun 30, 2016
weierophinney added a commit that referenced this pull request Jun 30, 2016
marshalBadControllerEvent argument type hinting
weierophinney added a commit that referenced this pull request Jun 30, 2016
weierophinney added a commit that referenced this pull request Jun 30, 2016
weierophinney added a commit that referenced this pull request Jun 30, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants