-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger] Allow any ServiceResetterInterface
implementation in ResetServicesListener
#60999
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
base: 7.4
Are you sure you want to change the base?
[Messenger] Allow any ServiceResetterInterface
implementation in ResetServicesListener
#60999
Conversation
c0f0ae3
to
aa6fb3b
Compare
Hi 👋 I hope you're all doing well! I think this kind of change is acceptable in a new major version (like now), but if I'm mistaken, I'm happy to update the PR. |
aa6fb3b
to
814a6d8
Compare
The change itself looks reasonable to me, but that's a new feature and we should merge it into the |
src/Symfony/Component/Messenger/Tests/EventListener/ResetServicesListenerTest.php
Outdated
Show resolved
Hide resolved
a240e9a
to
83714d6
Compare
Hi 👋 I hope you're all doing well! |
8.0 | ||
--- | ||
|
||
* Use `ServicesResetterInterface` instead of concrete `ServicesResetter` in `ResetServicesListener` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Use `ServicesResetterInterface` instead of concrete `ServicesResetter` in `ResetServicesListener` | |
* Allow any `ServiceResetterInterface` implementation in `ResetServicesListener` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the PR header to your suggestion @xabbuh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi 👋 I hope both of you are doing well ❤️
Thank you so much for the suggestion and the update to the PR header, i really appreciate it! ❤️
ServicesResetterInterface
instead of concrete ServicesResetter
in ResetServicesListener
ServiceResetterInterface
implementation in ResetServicesListener
…icesResetter` in `ResetServicesListener`
83714d6
to
93fdc14
Compare
Hey! 👋 The pipeline failure in the I see a few options we could take here:
if (interface_exists(ServicesResetterInterface::class)) {
// version using ServicesResetterInterface
} else {
// fallback using ServicesResetter
} This would keep compatibility with both versions and avoid raising the minimum requirement. Let me know which path you'd prefer, or if there's another alternative I haven't considered. Happy to adjust the PR accordingly! 🙏❤️ Edit: some resources |
bumping the require for the HttpKernel component makes sense to me |
Summary
This PR updates the
ResetServicesListener
to depend on theServicesResetterInterface
instead of the concreteServicesResetter
class.