Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[EventDispatcher] Fix TraceableEventDispatcher FC/BC layer#31254

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@chalasr
Copy link
Member

@chalasrchalasr commentedApr 25, 2019•
edited
Loading

QA
Branch?master
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?n/a
Tests pass?yes
Fixed tickets#31221
LicenseMIT
Doc PRn/a

also renamesWrappedEvent toLegacyEventProxy

yceruto reacted with heart emoji
Copy link
Member

@ycerutoyceruto left a comment

Choose a reason for hiding this comment

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

Yes, this solvesmy issue, thanks!

@chalasrchalasrforce-pushed theevent-prepostdispatch branch 2 times, most recently from0bef732 to1add058CompareApril 25, 2019 19:31
@chalasrchalasrforce-pushed theevent-prepostdispatch branch 2 times, most recently fromadef564 tofd24888CompareApril 25, 2019 21:23
@nicolas-grekas
Copy link
Member

See alternative in#31258

@chalasrchalasrforce-pushed theevent-prepostdispatch branch fromfd24888 to6a78e46CompareApril 26, 2019 21:27
};
}else {
$closure =$listenerinstanceof \Closure ?$listener : \Closure::fromCallable($listener);
$closure =$listenerinstanceof \Closure||$listenerinstanceof WrappedListener?$listener : \Closure::fromCallable($listener);
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

WrappedListener handles this optimization internally

@chalasr
Copy link
MemberAuthor

Remaining issue fixed, ready.

@chalasrchalasrforce-pushed theevent-prepostdispatch branch 2 times, most recently froma14a4ed toc34910eCompareApril 26, 2019 22:14
@xabbuh
Copy link
Member

What about also adding an assertion to the test which checks that the event instance passed to the listener actually is an instance of theContractsEvent?

@nicolas-grekas
Copy link
Member

the event instance passed to the listener actually is an instance of the ContractsEvent?

implementing this interface is not a requirement

@chalasrchalasrforce-pushed theevent-prepostdispatch branch fromc34910e toc5b3b34CompareApril 27, 2019 13:05
@chalasr
Copy link
MemberAuthor

Assertion added to ensure that the event passed to the listener is the same as the dispatched one (and not a proxy), also rebased to make the CI green.

xabbuh reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

Thank you@chalasr.

yceruto reacted with thumbs up emoji

@nicolas-grekasnicolas-grekas merged commitc5b3b34 intosymfony:masterApr 27, 2019
nicolas-grekas added a commit that referenced this pull requestApr 27, 2019
… (chalasr)This PR was merged into the 4.3-dev branch.Discussion----------[EventDispatcher] Fix TraceableEventDispatcher FC/BC layer| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | n/a| Tests pass?   | yes| Fixed tickets |#31221| License       | MIT| Doc PR        | n/aalso renames `WrappedEvent` to `LegacyEventProxy`Commits-------c5b3b34 [EventDispatcher] Fix TraceableEventDispatcher FC/BC layer
@chalasrchalasr deleted the event-prepostdispatch branchApril 27, 2019 13:43
@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@ycerutoycerutoyceruto approved these changes

+1 more reviewer

@SimperfitSimperfitSimperfit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

6 participants

@chalasr@nicolas-grekas@xabbuh@yceruto@Simperfit@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp