Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] removed the Doctrine Annotations lib dependency on FrameworkBundle#20097
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[FrameworkBundle] removed the Doctrine Annotations lib dependency on FrameworkBundle#20097
Uh oh!
There was an error while loading.Please reload this page.
Conversation
99cd189 toa7004d3Compare| ->arrayNode('annotations') | ||
| ->info('annotation configuration') | ||
| ->addDefaultsIfNotSet() | ||
| ->canBeDisabled() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Would be better for consistency to usecanBeEnabled, but that would be a too big BC break. At least with that default, Symfony is going to send a clear exception message about installingdoctrine/annotations. If not, people will get an error about an unknownannotation_reader service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks like we are already facing this. If I install framework-bundle + console directly and create a simple console application ontop of this - it should immediately fail as annotation not installed and the default configuration is to enable them.
Since these are the defaults - it is really not obviously that I have annotations enabled somewhere.
lemoinem commentedSep 30, 2016
Although I do understand the plan to reduce the amount of dependencies, and by no means intend to question the direction the symfony team is planning of going toward to. This said, annotations are currently recommended as Best Practices for Routing, ParamConverter and Doctrine Entities Mapping. They are a huge part of symfony-standard as it is currently used. Does this change intend to be a precursor of a change in the way the Annotations intend to be used in Symfony, starting at Symfony v4.0, or is it something that is intended only to involve the Framework Bundle when used as a library and not as part of the Symfony full stack and Standard Edition? |
fabpot commentedSep 30, 2016
As you can see, it is still enabled by default. The best practices and recommendations won't change (I'm anyway a big fan of using annotations). But, if we can make it optional for people not using annotations, that's a big win for everyone. |
6fcf19a to4bb158fCompare| 3.2.0 | ||
| ----- | ||
| * Removed`doctrine/annotations-core` from the list of required dependencies in`composer.json` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@fabpot it should bedoctrine/annotations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed
4bb158f toc2d8356Compare… dependency on FrameworkBundle (fabpot)This PR was merged into the 3.2-dev branch.Discussion----------[FrameworkBundle] removed the Doctrine Annotations lib dependency on FrameworkBundle| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | yes (fixing this is easy by adding doctrine/annotations explicitly)| Deprecations? | no| Tests pass? | yes| Fixed tickets |#15748 partially| License | MIT| Doc PR | n/aAnother PR to reduce the number of required dependencies on FrameworkBundle. This PR removes the Doctrine annotations library from the list.Commits-------c2d8356 [FrameworkBundle] removed the Doctrine Annotations lib dependency on FrameworkBundle
…s (ogizanagi)This PR was merged into the 3.2 branch.Discussion----------[FrameworkBundle] Smarter default for framework.annotations| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yesish (could be considered as a minor BC break)| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | N/A| License | MIT| Doc PR | N/A`framework.annotations` default should be true only if `doctrine/annotations` is installed.Indeed, in#20097, the dependency on `doctrine/annotations` was removed from the framework bundle.Thus, an application can break (not talking from one actually relying on annotations) as soon as it uses the framework bundle without the `framework.annotations` key explicitly set to `false` (I had the case in a fixture application in the testsuite of a package).Commits-------e38be09 [FrameworkBundle] framework.annotations default should be true only if doctrine/annotations is installed
…s (ogizanagi)This PR was merged into the 3.2 branch.Discussion----------[FrameworkBundle] Smarter default for framework.annotations| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yesish (could be considered as a minor BC break)| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | N/A| License | MIT| Doc PR | N/A`framework.annotations` default should be true only if `doctrine/annotations` is installed.Indeed, insymfony/symfony#20097, the dependency on `doctrine/annotations` was removed from the framework bundle.Thus, an application can break (not talking from one actually relying on annotations) as soon as it uses the framework bundle without the `framework.annotations` key explicitly set to `false` (I had the case in a fixture application in the testsuite of a package).Commits-------e38be09 [FrameworkBundle] framework.annotations default should be true only if doctrine/annotations is installed
Another PR to reduce the number of required dependencies on FrameworkBundle. This PR removes the Doctrine annotations library from the list.