drop annotation support with entities#1126
Conversation
composer.json
Outdated
| "conflict": { | ||
| "doctrine/orm": "<2.10" | ||
| "doctrine/orm": "<2.10", | ||
| "doctrine/doctrine-bundle": "<2.4" |
There was a problem hiding this comment.
2.4 introduces support for attributes... this allows use to remove some of our attribute conditionals. or at the very least - we would only need to verify the bundle is installed..
There was a problem hiding this comment.
side note - we already have doctrine-bundle: ^2.4 as a dev dependency.
jrushlow
force-pushed
the
legacy/doctrine
branch
2 times, most recently
from
8e958a8 to
b09fdd8
Compare
| if (!$this->doesEntityUseAttributeMapping($entityClassDetails->getFullName())) { | ||
| throw new RuntimeCommandException(sprintf('Only attribute mapping is supported by make:entity, but the <info>%s</info> class uses a different format. If you would like this command to generate the properties & getter/setter methods, add your mapping configuration, and then re-run this command with the <info>--regenerate</info> flag.', $entityClassDetails->getFullName())); |
There was a problem hiding this comment.
This is the one that we need to keep and determine if
- AttributeMapping is not used
- If somehow - AttributeMapping was being used but the class had
@ORM\Entity..type annotations.
Then we would throw this exception..
| return $this->doesClassUseDriver($className, AttributeDriver::class); | ||
| } | ||
|
|
||
| public function isDoctrineSupportingAttributes(): bool |
There was a problem hiding this comment.
After we remove that one if statement we talked about in this PR, is this used anymore?
There was a problem hiding this comment.
It won't be when we're all said and done, but make:user && make:reset-password are still calling on this.. they need to be refactored after this PR && the CSM one.
| } | ||
|
|
||
| /** @legacy Drop when Annotations are no longer supported */ | ||
| private function doesEntityUseAttributeMapping(string $className): bool |
There was a problem hiding this comment.
If we're keeping this, remove the comment above
There was a problem hiding this comment.
We shouldn't need to keep this. But for the same reasons above, we'll need to remove it in a final "clean up" PR.
| $this->addGetter( | ||
| $relation->getPropertyName(), | ||
| $relation->getCustomReturnType() ?: $typeHint, | ||
| $relation->getCustomReturnType() ?? $typeHint, |
There was a problem hiding this comment.
nit pick, but I don't think ?? is needed here
There was a problem hiding this comment.
I believe it still is because the return type for BaseRelation::getCustomReturnType(): ?string.
Before, we just had a "string" type hint instead of an actual return type. Now, we have to watch out for uninitialized properties. Side note: we're doing a lot of cleanup in CSM in the respective PR.
jrushlow
changed the title
weaverryan
force-pushed
the
legacy/doctrine
branch
from
f3024b0 to
e70cc83
Compare
|
Thanks Jesse! |
This PR was squashed before being merged into the 1.0-dev branch. Discussion ---------- [csm] strict typing && legacy code removal - drops CSM annotation support - cleans up legacy CSM code - implements strict typing - [x] Depends on #1126 Commits ------- ffc80c4 [csm] strict typing && legacy code removal
2 participants
No description provided.