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

[Messenger] Consume a PSR-14 dispatcher for dispatching events#45967

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

@derrabus
Copy link
Member

@derrabusderrabus commentedApr 7, 2022
edited
Loading

QA
Branch?6.1
Bug fix?no
New feature?yes
Deprecations?no
TicketsPart of#45963
LicenseMIT
Doc PRN/A

As@Crell said in#45963, requiring the event dispatcher passed to those classes to be an implementation of Symfony's contract was too restrictive. We can easily consume a PSR-14 dispatcher here.

This PR obviously only fixes the low hanging fruit. The commands coming with the messenger component register event subscribers on the dispatcher, in a way that we need the actual EventDispatcher component for.

Crell reacted with thumbs up emoji
@carsonbotcarsonbot added this to the5.4 milestoneApr 7, 2022
@derrabusderrabusforce-pushed thebugfix/messenger-psr-ed branch 2 times, most recently from3fcc3d5 toc3eedbfCompareApril 7, 2022 08:37
@derrabusderrabusforce-pushed thebugfix/messenger-psr-ed branch fromc3eedbf toa7794bbCompareApril 7, 2022 09:02
@derrabusderrabusforce-pushed thebugfix/messenger-psr-ed branch froma7794bb to5e04bf4CompareApril 7, 2022 10:15
"require": {
"php":">=7.2.5",
"psr/log":"^1|^2|^3",
"psr/event-dispatcher":"^1",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is technically only required if using the queue parts of the library, I think. If it's purely a sync message bus it's not required. That suggests it may be better as a suggests/dev.

PSR packages are small enough that it doesn't really matter, but it's worth mentioning as event-dispatcher wasn't in a first-level require. I'm comfortable either way.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

It was, because we could have Symfony's EventDispatcher contracts v1 installed which extended the PSR-14 interface conditionally (= only if it's present), mainly because of PHP 7.1 compatibility (PSR-14 requires PHP 7.2).

I have pushed a new version that should work even in that combination, which allowed me to remove the dependency again.

@derrabusderrabusforce-pushed thebugfix/messenger-psr-ed branch from5e04bf4 toa1213b0CompareApril 7, 2022 21:47
@fabpot
Copy link
Member

That's a new feature, and as such, it should target 6.1 (we merged the same for Mailer as a new feature as well).

derrabus reacted with thumbs up emoji

@derrabusderrabus changed the base branch from5.4 to6.1April 8, 2022 08:44
@derrabusderrabusforce-pushed thebugfix/messenger-psr-ed branch fromf1169bb to78603d0CompareApril 8, 2022 08:44
@derrabusderrabus added Feature and removed Bug labelsApr 8, 2022
@derrabus
Copy link
MemberAuthor

That's a new feature, and as such, it should target 6.1 (we merged the same for Mailer as a new feature as well).

All right. That makes it a lot easier. 😅

@derrabusderrabusforce-pushed thebugfix/messenger-psr-ed branch from78603d0 tof500c7aCompareApril 8, 2022 08:54
@nicolas-grekasnicolas-grekas modified the milestones:5.4,6.1Apr 11, 2022
@nicolas-grekas
Copy link
Member

Thank you@derrabus.

@nicolas-grekasnicolas-grekas merged commit8b680f0 intosymfony:6.1Apr 11, 2022
@derrabusderrabus deleted the bugfix/messenger-psr-ed branchApril 11, 2022 17:17
@fabpotfabpot mentioned this pull requestApr 15, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@HypeMCHypeMCHypeMC left review comments

@chalasrchalasrchalasr approved these changes

+1 more reviewer

@CrellCrellCrell left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

7 participants

@derrabus@fabpot@nicolas-grekas@Crell@HypeMC@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp