-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Document ORM drivers only really load ORM metadata #9677
Document ORM drivers only really load ORM metadata #9677
Conversation
d862703
to
ebfa2f5
Compare
ebfa2f5
to
c23b598
Compare
@@ -31,7 +31,7 @@ protected function convertToClassMetadata(array $entityTables, array $manyTables | |||
|
|||
$metadatas = []; | |||
foreach ($driver->getAllClassNames() as $className) { | |||
$class = new ClassMetadataInfo($className); | |||
$class = new ClassMetadata($className); |
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.
I first implemented this with assert
, then was forced to changed this test, which made me think this was a bit risky, hence the change towards phpdoc. It's better anyway, since it makes static analysis able to detect calls to these methods made with ClassMetadataInfo
rather than ClassMetadata
.
c23b598
to
85238d4
Compare
You're only removing |
Sorry, when I wrote this I had added some calls to |
Note: I'm not sure I made a great choice in doctrine/persistence#180 when I used templating in
MappingDriver
to bind$className
to$metadata
, as double templating does not seem possible, meaning we can't use it to get rid of the docblocks I added:Although this is clearly not a bugfix and touches
src
, I'm targeting 2.12.x becauseassert
is not supposed to affect production, and touching baseline files is best done on the lowest-supported branch IMO, so that there are not too many conflicts.