-
-
Notifications
You must be signed in to change notification settings - Fork 239
Add support for Symfony 8 #510
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: master
Are you sure you want to change the base?
Conversation
1f24bab
to
0236408
Compare
@@ -25,7 +25,7 @@ | |||
*/ | |||
class AddProcessorsPass implements CompilerPassInterface | |||
{ | |||
public function process(ContainerBuilder $container) | |||
public function process(ContainerBuilder $container): void |
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.
This is a breaking change. We should think about releasing a new major.
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.
Or use that trait trick you created :)
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.
This is a breaking change. We should think about releasing a new major.
Sure. If so, we could also think about removing support of sf < 6.4.
Or use that trait trick you created :)
What trait trick? I'm curious 🙂
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.
even the trait trick would be a BC break (but a conditional one, which is even worse to communicate)
However, given the time since the last major version, I think we could do one.
If so, we could also think about removing support of sf < 6.4.
this does not actually require a major version. We can do it in a minor version if we ensure we update the composer requirements properly (as composer will install compatible packages)
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.
Or use that trait trick you created :)
What trait trick? I'm curious 🙂
🙈 https://github.com/doctrine/dbal/blob/3.10.x/src/Tools/Console/Command/CommandCompatibility.php 🙈
I would prefer to go with a new major here in the case of our bundle.
Add support of Symfony 8 and fixes typehint errors like