Skip to content

[EventDispatcher] Better error reporting when arguments to dispatch() are swapped #34677

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

Merged
merged 1 commit into from
Nov 28, 2019
Merged

[EventDispatcher] Better error reporting when arguments to dispatch() are swapped #34677

merged 1 commit into from
Nov 28, 2019

Conversation

rimas-kudelis
Copy link

@rimas-kudelis rimas-kudelis commented Nov 28, 2019

Q A
Branch? 4.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #34676
License MIT

Incorrect error is currently being reported when the second argument is an instance of Symfony\Contracts\EventDispatcher\Event.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LegacyEventDispatcherProxy also needs a similar patch

@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Nov 28, 2019
@nicolas-grekas nicolas-grekas changed the title Better error reporting when arguments to dispatch() are swapped [EventDispatcher] Better error reporting when arguments to dispatch() are swapped Nov 28, 2019
nicolas-grekas
nicolas-grekas previously approved these changes Nov 28, 2019
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Nov 28, 2019

It looks like we broke a test case sorry.

@rimas-kudelis
Copy link
Author

Should I revert to checking for actual classes then, or change tests instead?

@nicolas-grekas
Copy link
Member

revert to checking for actual classes

yes please

@rimas-kudelis
Copy link
Author

Thanks for bearing with me! I can squash the commits if you like.

@nicolas-grekas
Copy link
Member

Thank you @rimas-kudelis.

nicolas-grekas added a commit that referenced this pull request Nov 28, 2019
… dispatch() are swapped (rimas-kudelis)

This PR was squashed before being merged into the 4.3 branch.

Discussion
----------

[EventDispatcher] Better error reporting when arguments to dispatch() are swapped

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34676
| License       | MIT

Incorrect error is currently being reported when the second argument is an instance of `Symfony\Contracts\EventDispatcher\Event`.

Commits
-------

54aac56 [EventDispatcher] Better error reporting when arguments to dispatch() are swapped
@nicolas-grekas nicolas-grekas merged commit 54aac56 into symfony:4.3 Nov 28, 2019
@rimas-kudelis rimas-kudelis deleted the 4.3-patch2 branch November 28, 2019 11:55
This was referenced Dec 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants