Skip to content
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

Deprecate the Annotation interface #10178

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

derrabus
Copy link
Member

Once we remove the support for annotations, having a marker interface named Annotation for our attribute classes seems weird. Let's rename it.

@derrabus derrabus added this to the 2.14.0 milestone Oct 26, 2022
@derrabus derrabus requested a review from greg0ire October 26, 2022 07:59
@derrabus derrabus mentioned this pull request Oct 26, 2022
@derrabus derrabus force-pushed the deprecate/annotation-interface branch from ef380ae to ead2f90 Compare October 26, 2022 08:04
@@ -39,6 +39,8 @@
<referencedClass name="Doctrine\ORM\Tools\Console\Command\GenerateRepositoriesCommand"/>
<referencedClass name="Doctrine\ORM\Tools\Console\Helper\EntityManagerHelper"/>
<referencedClass name="Doctrine\ORM\Tools\Console\EntityManagerProvider\HelperSetManagerProvider"/>
<!-- https://github.com/vimeo/psalm/issues/8617 -->
<referencedClass name="Doctrine\ORM\Mapping\Annotation"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface is reported as a class when used in generics. I've created vimeo/psalm#8617 to track this issue.

namespace Doctrine\ORM\Mapping;

/** A marker interface for mapping attributes. */
interface MappingAttribute extends Annotation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you name it MappingAttribute and not just Attribute? It is already inside a Mapping namespace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because literally all classes that implement this interface also have an attribute attached to them named Attribute and I didn't want to screw up the imports everywhere. I also wanted the name of the marker interface to be a bit more specific. After all, it's not just an arbitrary attribute, it's one that we use to configure mapping information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thanks!

@derrabus derrabus merged commit 543be3f into doctrine:2.14.x Oct 26, 2022
@derrabus derrabus deleted the deprecate/annotation-interface branch October 26, 2022 19:51
derrabus added a commit to derrabus/orm that referenced this pull request Oct 31, 2022
* 2.14.x:
  Deprecate the Annotation interface (doctrine#10178)
  Bump CI to PHP 8.2 and latest database versions (doctrine#10180)
  Remove reference to deprecated DriverChain from docs (doctrine#10179)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants