Skip to content

[Notifier] Prepare bridges for the upcoming return type change. #39558

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
Dec 18, 2020

Conversation

derrabus
Copy link
Member

Q A
Branch? 5.2
Bug fix? no
New feature? no
Deprecations? no
Tickets N/A
License MIT
Doc PR N/A

Preparation for #39549.

@nicolas-grekas
Copy link
Member

we should do this in all bridges actually

@derrabus
Copy link
Member Author

Even if the others aren't affected by the BC break?

@nicolas-grekas
Copy link
Member

They can be affected by any BC break we might want to do in 5.3+

@derrabus
Copy link
Member Author

All right.

@nicolas-grekas
Copy link
Member

Thank you @derrabus.

@nicolas-grekas nicolas-grekas merged commit 8716049 into symfony:5.2 Dec 18, 2020
@derrabus
Copy link
Member Author

I'll simply discard the changes when merging up, right?

@derrabus derrabus deleted the bugfix/return-type branch December 18, 2020 12:43
@stof
Copy link
Member

stof commented Dec 18, 2020

I already suggested that in the past when dealing with another experimental component (Messenger or Mailer, I don't remember). IMO, dependencies to an experimental component should never be added using the ^ operator, because marking a component as experimental is precisely about not following semver yet in it (which is the requirement for ^ to be safe).

@nicolas-grekas
Copy link
Member

We did so in 5.1 for notifier, and we lost the practice when preparing 5.2.
Now merged up to 5.3, using ~5.3.0

@derrabus
Copy link
Member Author

I've bumped the conflict rules as well: #39559

@stof
Copy link
Member

stof commented Dec 18, 2020

indeed, maybe it was notifier itself in 5.1 then 😄

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.

4 participants