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

[Mailer] Changed EventDispatcherInterface dependency from Component to Contracts#31956

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

@Koc
Copy link
Contributor

@KocKoc commentedJun 8, 2019
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?no
BC breaks?yes
Deprecations?no
Tests pass?almost yes, see#31956 (comment)
Fixed tickets-
LicenseMIT
Doc PR-

Follow up of#31946 (comment) . I hope this kind of changes are allowed for experimental components.

BTW,@nicolas-grekas , why Psr14 interface is optional for Contract's `EventDispatcherInterfacehttps://github.com/symfony/symfony/blob/4.4/src/Symfony/Contracts/EventDispatcher/EventDispatcherInterface.php#L16 ?

@KocKocforce-pushed themailer-change-event-dispatcher-dependency branch frome3efc9e to848fce2CompareJune 8, 2019 12:53
@nicolas-grekasnicolas-grekas added this to thenext milestoneJun 8, 2019
@nicolas-grekasnicolas-grekas changed the titleChanged EventDispatcherInterface dependency from Component to Contracts[Mailer] Changed EventDispatcherInterface dependency from Component to ContractsJun 8, 2019
@nicolas-grekas
Copy link
Member

Makes sense to me, don't miss bumping the bridge's lowest version in composer.json

why Psr14 interface is optional for Contract's `EventDispatcherInterface

technically: because we are >=PHP7.1 while PSR14 is >=7.2
also because PSR14 doesn't need to be a hard requirement.

@KocKocforce-pushed themailer-change-event-dispatcher-dependency branch fromd39cada tobdb6217CompareJune 9, 2019 08:03
@Koc
Copy link
ContributorAuthor

Koc commentedJun 9, 2019

@nicolas-grekas I've updated minimum required versions for both Bridge -> Mailer and Mailer -> Bridge, but deps=high job still fails because current dev-master hasn't my changes. How can I fix that or should we just ignore this failed job?

@nicolas-grekas
Copy link
Member

How can I fix that or should we just ignore this failed job?

deps=high can fail in this situation, until the patch is merged to master.

@fabpot
Copy link
Member

Thank you@Koc.

@fabpotfabpot merged commitbdb6217 intosymfony:4.4Jun 11, 2019
fabpot added a commit that referenced this pull requestJun 11, 2019
…rom Component to Contracts (Koc)This PR was merged into the 4.4 branch.Discussion----------[Mailer] Changed EventDispatcherInterface dependency from Component to Contracts| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| BC breaks?    | yes| Deprecations? | no| Tests pass?   | almost yes, see#31956 (comment)| Fixed tickets | -| License       | MIT| Doc PR        | -Follow up of#31946 (comment) . I hope this kind of changes are allowed for experimental components.BTW,@nicolas-grekas , why Psr14 interface is optional for Contract's `EventDispatcherInterfacehttps://github.com/symfony/symfony/blob/4.4/src/Symfony/Contracts/EventDispatcher/EventDispatcherInterface.php#L16 ?Commits-------bdb6217 Changed EventDispatcherInterface dependency from Component to Contracts
4.4.0
-----

*[BC BREAK] Transports depend on`Symfony\Contracts\EventDispatcher\EventDispatcherInterface`
Copy link
Member

Choose a reason for hiding this comment

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

that's not actually a BC break if the component interface extends the contracts one.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Interface from component has methods likeaddListener,addSubscriber but interface from contracts has onlydispatch method. So if some of transports calladdListener - now it could get fatal error.

@KocKoc deleted the mailer-change-event-dispatcher-dependency branchJune 11, 2019 20:13
@nicolas-grekasnicolas-grekas modified the milestones:next,4.4Oct 27, 2019
This was referencedNov 12, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@Koc@nicolas-grekas@fabpot@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp