Custom collection support #181
Conversation
|
Hey @alcaeus. Just wanted to drop a note to say thanks for taking care of this, and also, for keeping the other things like the test changes in tact! 👍 |
| @@ -31,6 +31,12 @@ | |||
| 'hydrator_dir' => 'data/DoctrineMongoODMModule/Hydrator', | |||
| 'hydrator_namespace' => 'DoctrineMongoODMModule\Hydrator', | |||
|
|
|||
| 'generate_persistent_collections' => Configuration::AUTOGENERATE_ALWAYS, | |||
There was a problem hiding this comment.
Out of scope for this PR, but is it common for other Zend modules to use AUTOGENERATE_ALWAYS for proxies and hydrators? ODM bundle for instance does the opposite relying on generating proxies/hydrators/collections during cache warmup. Maybe it's something that could be improved for 1.0.0?
There was a problem hiding this comment.
What do you mean by, other Zend moduels? This is the Zend module for Doctrine integration. I don't think AUTOGENERATE_NEVER is a good default since it means your application is broken by default. That's a production default, not a developer default.
There was a problem hiding this comment.
I mean other Doctrine-related Zend modules. And I must disagree that app is broken by default, in Symfony's case when cache is not there, it's generated and so are proxies/hydrators because that's hooked into cache-warming process by the bundle so the app is never broken by default.
There was a problem hiding this comment.
I do not believe ZF has any such thing as a "cache-warming process". One would have to generate Doctrine assets manually, which is not a good default for development.
| @@ -162,8 +197,8 @@ public function getDriver() | |||
|
|
|||
| /** | |||
| * | |||
| * @param boolean|int $generateProxies | |||
| * @return \DoctrineMongoODMModule\Options\Configuration | |||
| * @param boolean $generateProxies | |||
There was a problem hiding this comment.
This should be int and for BC there could be something like in bundle: https://github.com/doctrine/DoctrineMongoDBBundle/blob/ee1cdebc02d4ed49e90e9ad15737b1be03ac3eee/DependencyInjection/Configuration.php#L54-L58
alcaeus
force-pushed
the
custom-collection-support
branch
from
c308cec
to
ce96953
Compare
Supersedes #166 and #165. I took the liberty of cherry-picking the commits containing the feature and left out larger refactorings (except for
ConfigurationFactoryTestwhich makes sense even at this point). I'll be cherry-picking refactorings into a separate PR along with #175.Thanks @Bilge for doing the heavy lifting on this one!