Conversation
|
Also, while you're fixing up the README, maybe you could fix these too: |
mathroc
left a comment
mathroc
left a comment
There was a problem hiding this comment.
just left a bunch of suggestions, feel free to pick what's interesting if any and discard the rest
| * | ||
| * @param string $tag | ||
| * The tag for which we want relevant Listeners. | ||
| * @return iterable |
There was a problem hiding this comment.
| * @return iterable<callable(object)> |
| abstract protected function getListenersForAllTags() : iterable; | ||
|
|
||
| /** | ||
| * @return iterable<callable> |
There was a problem hiding this comment.
| * @return iterable<callable> | |
| * @template T | |
| * @param T $event | |
| * @return iterable<callable(T)> |
| * tag-specific Listeners will always be invoked first. | ||
| * | ||
| * @return iterable | ||
| * @return iterable<callable> |
There was a problem hiding this comment.
| * @return iterable<callable> | |
| * @return iterable<callable(object)> |
| * @param callable $callable | ||
| * @return bool | ||
| * True if the callable represents a static method, false otherwise. | ||
| */ |
There was a problem hiding this comment.
| * @psalm-assert-if-true list{class-string, ...} $callable | |
| */ |
| // See the docblock of isClassCallable() for why this needs to come first. | ||
| if ($this->isClassCallable($callable)) { | ||
| $reflect = new \ReflectionClass($callable[0]); | ||
| $params = $reflect->getMethod($callable[1])->getParameters(); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -35,8 +35,7 @@ | |||
|
|
|||
| $rType = $params[0]->getType(); | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
I suspect there's a decent amount of refactoring and error handling that could be done here, but that's out of scope for this PR. I just wanted to hit some low-hanging fruit before tagging the new 8.1-specific release. If you want to try improving that, though, follow-on PRs are welcome.
(For that matter, it's possible we could simplify most of it with Closure::fromCallable()? Dunno.)
| * @param object $event | ||
| * The Event to match against. | ||
| * @param iterable $listenerSet | ||
| * An iterable in the format returned by getListenersForTag()/getListenersForAllTags(). | ||
| * @return iterable | ||
| * @return iterable<callable> |
There was a problem hiding this comment.
| * @return iterable<callable> | |
| * @template T | |
| * @param object $event | |
| * The Event to match against. | |
| * @param iterable<iterable<T>> $listenerSet | |
| * An iterable in the format returned by getListenersForTag()/getListenersForAllTags(). | |
| * @return iterable<T> |
| * | ||
| * @param callable $callable | ||
| * @return bool | ||
| * True if the callable represents an invokable object, false otherwise. |
There was a problem hiding this comment.
| * True if the callable represents an invokable object, false otherwise. | |
| * @psalm-assert-if-true object $callable |
|
I'm going to pass on most of the additional generics and Psalm stuff. Using tool-specific tags is a larger question for FIG, and I'd want to spend enough time on it to make sure all the generics are correct. And right now this project doesn't even have an SA configured. 😄 I'll merge this tomorrow and tag a release unless there's other feedback specifically on the changes in the patch already. |
4 participants
Bump some versions, improve docs, and add some types. Preparing for the next tagged release.