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.

Sign up for GitHub

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

Jump to bottom

Add support for Doctrine ORM 3 #34

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Add support for Doctrine ORM 3 #34

wants to merge 3 commits into from

Conversation

NanoSector
Copy link
Contributor

@NanoSector NanoSector commented Jun 28, 2024 •
edited
Loading

This allows installation on Doctrine ORM 3 and adjusts an overridden method for ORM 3 compatibility. Fixes #29

This is untested on ORM 2.

I am unsure how you'd like to handle this upgrade. In my opinion this can go either of two ways:

  1. We use the Symfony service container to dynamically swap between two implementations depending on what version is installed,
  2. We refactor the code to not override Doctrine methods to ensure compatibility (using middlewares instead?)

What do you think?

@NanoSector
Copy link
Contributor Author

@debesha Sorry to bother you but have you had a chance to take a look at this?

@debesha
Copy link
Owner

debesha commented Jul 10, 2024

This pull request is still a work in progress
Draft pull requests cannot be merged.

@NanoSector
Copy link
Contributor Author

@debesha Thank you, I've marked this PR as a draft because there is still the open question on how to safely perform this upgrade.

Since updating the class with new parameter types might break the ORM 2.x compatibility (which we have not been able to test as we have no ORM 2.x project), I'd be a bit hesitant to immediately merge this into a release.

@debesha
Copy link
Owner

debesha commented Jul 10, 2024

Unfortunately, I won't be able to make a proper investigation on how is it safe to merge the PR

@NanoSector
Copy link
Contributor Author

Understandable. I think a good course of action would be to include some tests which test that the bundle can be integrated properly into various Symfony + Doctrine combinations; perhaps testing against Symfony 5.4 + ORM 2.x and Symfony 7.1 + ORM 3 is a good start.

I'll have a look later this week if I get the chance

@janklan
Copy link
Contributor

janklan commented Sep 16, 2024

Unfortunately, I won't be able to make a proper investigation on how is it safe to merge the PR

Whoa, Ok, so why did you approve it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@debesha debesha debesha approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: support for doctrine/orm to v3
3 participants
@NanoSector @debesha @janklan