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

Fix registering doctrine subscriber for kernel 6 with doctrine-bridge 7 #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

core23
Copy link

@core23 core23 commented Mar 5, 2024

Fixes #28 when using symfony/doctrine-bridge v7 with symfony/http-kernel v6.

The doctrine.event_subscriber is deprecated since symfony 6.3:

User Deprecated: Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead

@core23
Copy link
Author

core23 commented Mar 5, 2024

Can you please double check @Zombaya?

@Zombaya
Copy link

Zombaya commented Mar 7, 2024

I'm afraid I will not merge your MR, since in my opinion it's trying to solve the wrong problem(s).

1. Mixed symfony-versions are not recommended

You should not be using multiple versions of symfony together. The recommended way to do this is to use symfony/flex, which is available since symfony 4.0.

I'm not sure where it is documented on how to enable it, but it shows on the documention on how to upgrade a major version how you can upgrade from one version to the next:

# composer.json

"extra": {
      "symfony": {
           "allow-contrib": false,
-          "require": "6.4.*"
+          "require": "7.0.*"
      }
  }

2. Symfony 7.x should use attributes

When using symfony 7.x, we will be using attributes and will register ourselves as listeners instead of subscribers. We will not trigger the deprecation notice.

Conclusion

As I think this problem will be solved when using a single version of symfony, I will not be merging in this MR.

If this problem would trigger when exclusively using symfony 6.x, then I'd be willing to re-investigate.

@Zombaya Zombaya mentioned this pull request Mar 7, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants