[FrameworkBundle] removed the Doctrine Annotations lib dependency on FrameworkBundle #20097
Conversation
99cd189 to
a7004d3
Compare
| @@ -593,7 +593,7 @@ private function addAnnotationsSection(ArrayNodeDefinition $rootNode) | |||
| ->children() | |||
| ->arrayNode('annotations') | |||
| ->info('annotation configuration') | |||
| ->addDefaultsIfNotSet() | |||
| ->canBeDisabled() | |||
There was a problem hiding this comment.
Would be better for consistency to use canBeEnabled, but that would be a too big BC break. At least with that default, Symfony is going to send a clear exception message about installing doctrine/annotations. If not, people will get an error about an unknown annotation_reader service.
There was a problem hiding this comment.
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.
|
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? |
|
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 to
4bb158f
Compare
| @@ -4,6 +4,7 @@ CHANGELOG | |||
| 3.2.0 | |||
| ----- | |||
|
|
|||
| * Removed `doctrine/annotations-core` from the list of required dependencies in `composer.json` | |||
There was a problem hiding this comment.
@fabpot it should be doctrine/annotations
4bb158f to
c2d8356
Compare
… 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/a Another 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, in symfony/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.