Skip to content
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

Listener factory and checker #22

Merged
merged 9 commits into from Feb 26, 2021
Merged

Conversation

viktorprogger
Copy link
Contributor

@viktorprogger viktorprogger commented Feb 19, 2021

Q A
Is bugfix?
New feature? ✔️
Breaks BC? ✔️
Fixed issues

This PR is evolution of yiisoft/event-dispatcher#55
Support of invokale objects is added.

Corresponding PRs in the other packages should be merged after this one:

@viktorprogger viktorprogger requested review from samdark and a team February 19, 2021 16:25
@viktorprogger viktorprogger added the status:under development Someone is working on a pull request. label Feb 19, 2021
@samdark
Copy link
Member

samdark commented Feb 19, 2021

@viktorprogger is it possible to integrate ListenerConfigurationChecker somewhere so it's actually used?

@viktorprogger
Copy link
Contributor Author

viktorprogger commented Feb 20, 2021

@samdark, yes, there will be sattelite PRs in the demo and templates.

@viktorprogger viktorprogger added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Feb 25, 2021
src/ListenerCollectionFactory.php Outdated Show resolved Hide resolved
src/ListenerConfigurationChecker.php Outdated Show resolved Hide resolved

throw new InvalidEventConfigurationFormatException(
"Event listeners for $eventName must be an array, $type given."
"Event listeners for $eventName must be an iterable, $type given."
Copy link
Member

Choose a reason for hiding this comment

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

Such check exists in ListenerConfigurationChecker. Should we duplicate it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, 4 lines after this one we do foreach ($listeners.... We just throw an exception instead of php error.

src/ListenerConfigurationChecker.php Outdated Show resolved Hide resolved
src/ListenerConfigurationChecker.php Outdated Show resolved Hide resolved
viktorprogger and others added 2 commits February 26, 2021 11:07
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
@viktorprogger viktorprogger requested a review from a team February 26, 2021 14:05
@samdark samdark merged commit e89aaed into master Feb 26, 2021
@samdark samdark deleted the listener-factory-and-checker branch February 26, 2021 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:code review The pull request needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants