Skip to content
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

Drop support for Symfony 4 #1586

Merged
merged 1 commit into from Dec 3, 2022
Merged

Conversation

derrabus
Copy link
Member

Active support for Symfony 4 ends this month. This PR drops compat code we have maintained for Symfony 4.

@derrabus derrabus force-pushed the drop/symfony-4 branch 4 times, most recently from 5f3a918 to 94a4fa8 Compare November 29, 2022 22:03
@dmaicher
Copy link
Contributor

I think we should then also remove all of those?

https://github.com/doctrine/DoctrineBundle/blob/2.8.x/Resources/config/dbal.xml#L26-L37

This would then also fix #1585

@dmaicher dmaicher added this to the 2.8.0 milestone Nov 30, 2022
@derrabus
Copy link
Member Author

I think we should then also remove all of those?

There are tests in place that assert that the doctrine.dbal.logger service exists even if we use the logger middleware. But of course, that service serves no purpose anymore. It's just there. If removing it is not considered a BC break, I can remove it and adjust the tests.

@ostrolucky
Copy link
Member

People will be upset about removing this service for sure, but better to throw it in their face that they need to change something, rather than their code not doing anything. So I agree with removal.

@stof
Copy link
Member

stof commented Nov 30, 2022

Symfony supports marking a service as deprecated. That's what we should do for the logger service when we use the middleware

@dmaicher
Copy link
Contributor

dmaicher commented Dec 1, 2022

Symfony supports marking a service as deprecated. That's what we should do for the logger service when we use the middleware

But with this PR we will always use the middlewares. So the legacy logging services are not doing anything anymore. You would prefer to still keep them and deprecate them?

EDIT: I personally would be fine with removing them. I see those as internal details and not as an extension point that one should rely on.

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.

None yet

4 participants