Skip to content

[DoctrineBridge] Require doctrine/persistence 2 #39118

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
Merged
merged 1 commit into from
Nov 27, 2020
Merged

[DoctrineBridge] Require doctrine/persistence 2 #39118

merged 1 commit into from
Nov 27, 2020

Conversation

greg0ire
Copy link
Contributor

Q A
Branch 5.x
Bug fix no
New feature no
Deprecations no
Tickets n/a
License MIT
Doc PR n/a

This allows us to remove autoload calls that are necessary for the
persistence 1 backwards-compatibility layer to work.

This is a follow up of #35728

@stof
Copy link
Member

stof commented Nov 19, 2020

As there is no change in the composer.json of the doctrine bridge, you have no guarantee that doctrine/persistence 2 is actually used when using the subtree packages.

@greg0ire greg0ire force-pushed the cleanup-autoload-calls branch from e3d866e to 418da2b Compare November 19, 2020 18:55
@derrabus
Copy link
Member

Making Persistence 2.0 a requirement would basically render our codebase incompatible with ORM 2.6, correct?

@stof
Copy link
Member

stof commented Nov 19, 2020

ORM 2.6 is EOL AFAICT

@greg0ire
Copy link
Contributor Author

I think it would, but I don't think it's a big deal, for reasons pointed out by @stof

@derrabus
Copy link
Member

Yeah, I think we're good. It's just that the Bridge's composer.json says "doctrine/orm": "^2.6.3" and I thought we should've at least talked about that hidden version bump. 😃

@stof
Copy link
Member

stof commented Nov 19, 2020

@alcaeus is the ODM 1.3.x still maintained (and supported by a version of the bundle compatible with Symfony 5) ? Because that would also drop support for the ODM 1.3.x

@stof
Copy link
Member

stof commented Nov 19, 2020

It's just that the Bridge's composer.json says "doctrine/orm": "^2.6.3" and I thought we should've at least talked about that hidden version bump.

We could update that one to ^2.7.3 to avoid confusion

@greg0ire
Copy link
Contributor Author

Should I bump the ORM constraint globally as well?

@derrabus
Copy link
Member

@greg0ire Yes, please. The ORM version referenced there can be considered antique by now. 😅

@greg0ire greg0ire force-pushed the cleanup-autoload-calls branch from 418da2b to 1fcf6fa Compare November 19, 2020 19:09
@alcaeus
Copy link
Contributor

alcaeus commented Nov 19, 2020

@alcaeus is the ODM 1.3.x still maintained (and supported by a version of the bundle compatible with Symfony 5) ? Because that would also drop support for the ODM 1.3.x

It's no longer supported, so we can start the process of dropping support for it.

fabpot
fabpot approved these changes Nov 20, 2020
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about doing it in the 5.2 branch instead?

@greg0ire greg0ire changed the base branch from 5.x to 5.2 November 20, 2020 08:22
@greg0ire
Require doctrine/persistence 2
574a184 
This allows us to remove autoload calls that are necessary for the
persistence 1 backwards-compatibility layer to work.
The require-dev constraints on doctrine/orm are bumped to ^2.7.3 because
lower versions are not compatible with doctrine/persistence 2.
@greg0ire greg0ire force-pushed the cleanup-autoload-calls branch from 1fcf6fa to 574a184 Compare November 20, 2020 08:22
@greg0ire
Copy link
Contributor Author

@fabpot you're right, done!

@chalasr
Copy link
Member

chalasr commented Nov 20, 2020

Build failure looks unrelated but interesting https://travis-ci.com/github/symfony/symfony/jobs/444188596#L2509
//cc @xabbuh @nicolas-grekas

@nicolas-grekas
Copy link
Member

fix welcome :)

@nicolas-grekas nicolas-grekas added this to the 5.2 milestone Nov 20, 2020
@carsonbot carsonbot changed the title Require doctrine/persistence 2 [DoctrineBridge] Require doctrine/persistence 2 Nov 21, 2020
@fabpot
Copy link
Member

fabpot commented Nov 27, 2020

Thank you @greg0ire.

@fabpot fabpot mentioned this pull request Nov 27, 2020
Closed
@fabpot fabpot merged commit ac25e8c into symfony:5.2 Nov 27, 2020
@greg0ire greg0ire deleted the cleanup-autoload-calls branch November 27, 2020 06:53
@fabpot fabpot mentioned this pull request Nov 30, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@stof stof stof approved these changes

@derrabus derrabus derrabus approved these changes

@chalasr chalasr chalasr approved these changes

@wouterj wouterj Awaiting requested review from wouterj wouterj is a code owner

@sroze sroze Awaiting requested review from sroze

@xabbuh xabbuh Awaiting requested review from xabbuh

@yceruto yceruto Awaiting requested review from yceruto

Assignees
No one assigned
Labels
DoctrineBridge Status: Needs Review
Projects
None yet
Milestone
5.2
Development

Successfully merging this pull request may close these issues.
8 participants
@greg0ire @stof @derrabus @alcaeus @chalasr @nicolas-grekas @fabpot @carsonbot