Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Bug fixes 2012/10 #9

Merged
merged 7 commits into from Nov 19, 2012

Conversation

Projects
None yet
4 participants
Contributor

jonmchan commented Oct 3, 2012

Here's the changes that I had to fix to get it working with the latest dev version of doctrine/common. I also updated the README to reflect code changes and to add clarification on how to use the EntityManager.

@stof stof and 2 others commented on an outdated diff Oct 4, 2012

$cache = new ArrayCache;
- $metadata = new AnnotationDriver(new AnnotationReader);
- $entityManager = new EntityManager($storage, $cache, $metadata);
+ $storage = $storage ?: new DoctrineCacheStorage($cache);
@stof

stof Oct 4, 2012

Member

If you do it this way, you will have an error about reading an undefined variable $storage. What is the goal of adding a ternary operator here ?

@jonmchan

jonmchan Oct 9, 2012

Contributor

this was simply copied and pasted from the KeyValueStoreTestCase. If there is an issue with it here, the Tests should probably be updated too.

https://github.com/doctrine/KeyValueStore/blob/master/tests/Doctrine/Tests/KeyValueStoreTestCase.php

@Baachi

Baachi Oct 9, 2012

Contributor

That's not true.
The KeyValueStoreTestCase has a $storage argument, so no warning will be shown.

@jonmchan

jonmchan Oct 12, 2012

Contributor

you are right, erroneous copy and paste. Corrected.

@stof stof commented on an outdated diff Oct 4, 2012

...ctrine/KeyValueStore/Mapping/ClassMetadataFactory.php
@@ -94,5 +94,13 @@ protected function initializeReflection(ClassMetadata $class, ReflectionService
}
}
}
+
+ /**
+ * copied from doctrine/common - tests/Doctrine/Tests/Common/Persistence/Mapping/ClassMetadataFactoryTest.php
+ */
+ protected function isEntity(ClassMetadata $class)
+ {
+ return isset($class->isMappedSuperclass) && $class->isMappedSuperclass === false;
@stof

stof Oct 4, 2012

Member

KeyValueStore does not support mapped superclass. So any mapped class shuld be considered as an entity. This method should simply return true in all cases.

beberlei added a commit that referenced this pull request Nov 19, 2012

@beberlei beberlei merged commit f942e51 into doctrine:master Nov 19, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment