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

Mark doctrine/annotations dependency as optional #197

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

greg0ire
Copy link
Member

Now that the ORM has a driver based on PHP attributes, it seems less
necessary to require doctrine/annotations. In fact, that could also be
considered true before since there are already other drivers than the
annotation driver.
Fixes #195

@greg0ire
Copy link
Member Author

Cc @javiereguiluz 🙂

alcaeus
alcaeus previously approved these changes Jun 25, 2021
composer.json Outdated Show resolved Hide resolved
Now that the ORM has a driver based on PHP attributes, it seems less
necessary to require doctrine/annotations. In fact, that could also be
considered true before since there are already other drivers than the
annotation driver.
Fixes doctrine#195
@greg0ire greg0ire force-pushed the optional-doctrine-annotations branch from 1fe2328 to 52ab59d Compare June 25, 2021 18:53
@greg0ire greg0ire merged commit 97b7243 into doctrine:2.3.x Jun 25, 2021
@greg0ire greg0ire deleted the optional-doctrine-annotations branch June 25, 2021 18:56
@greg0ire greg0ire added this to the 2.3.0 milestone Jun 25, 2021
deek87 added a commit to deek87/concretecms that referenced this pull request Jan 20, 2022
add doctrine/annotations as required dependency
see doctrine/persistence#197
deek87 added a commit to deek87/concretecms that referenced this pull request Jan 20, 2022
add doctrine/annotations as required dependency
see doctrine/persistence#197
KorvinSzanto pushed a commit to concretecms/concretecms-core that referenced this pull request Jan 20, 2022
add doctrine/annotations as required dependency
see doctrine/persistence#197
KorvinSzanto pushed a commit to concretecms/concretecms-core that referenced this pull request Jan 20, 2022
add doctrine/annotations as required dependency
see doctrine/persistence#197
@alexislefebvre
Copy link

alexislefebvre commented Jan 31, 2022

If we upgrade from 2.2.4 to 2.3.0, doctrine/annotations is not installed anymore. So that Doctrine\Common\Annotations\AnnotationReader is not available anymore. Is it a BC break or a weak dependency in the projects that relied on AnnotationReader through doctrine/persistence?

Here is how I fixed that issue in one project: https://github.com/liip/LiipTestFixturesBundle/pull/170/files

@derrabus
Copy link
Member

If your project relies on Doctrine Annotations for metadata mapping, adding the explicit dependency is the way to go.

@derrabus
Copy link
Member

Related: doctrine/orm#9452

@alexislefebvre
Copy link

alexislefebvre commented Jan 31, 2022

Thanks, so I wonder how AnnotationReader can be available here in FrameworkBundle when doctrine/annotations is only a dev dependency

Update: my bad, it's a dev dependency since Symfony 4.4 at least, so this is not related to this PR.

@derrabus
Copy link
Member

A use statement alone does not require the referenced class to exist.

@alexislefebvre
Copy link

I'll have to investigate on Symfony's side, but I ended up with a annotations.reader service that was missing because Doctrine\Common\Annotations\AnnotationReader was not installed.

@derrabus
Copy link
Member

Yes. If the library is missing, Symfony won't create that service. But let's not flood this PR with chat messages. Feel free to open a discussion on either board, if you have further questions:

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.

Don't define Doctrine annotations as a hard dependency?
5 participants