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

Use proxy class name resolvers instead of local workaround #2304

Merged
merged 3 commits into from
May 16, 2021

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented May 15, 2021

Q A
Type improvement
BC Break no
Fixed issues

Summary

With the release of doctrine/persistence 2.2, we can now leverage class name resolvers in the metadata factory directly and can remove our workaround wherever we fetched metadata.

@alcaeus alcaeus added this to the 2.3.0 milestone May 15, 2021
@alcaeus alcaeus requested a review from malarzm May 15, 2021 19:10
@alcaeus alcaeus self-assigned this May 15, 2021
/**
* Returns the class name resolver which is used to resolve real class names for proxy objects.
*
* @deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Shall we point user to what they should be using now? What about a runtime deprecation?

Copy link
Member Author

Choose a reason for hiding this comment

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

That should've been marked internal and people shouldn't use it. I'll add a note in the upgrade file

/**
* @internal
*/
final class CachingClassNameResolver implements ClassNameResolver
final class CachingClassNameResolver implements ClassNameResolver, ProxyClassNameResolver
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this implementation be also moved to doctrine/common? I imagine all out libraries will want to have one

Copy link
Member Author

Choose a reason for hiding this comment

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

I can move it to doctrine/common, but that would delay this change. Since this class is internal, I'll mess with it in as many minor releases as I can.

@@ -4,6 +4,7 @@

namespace Doctrine\ODM\MongoDB\Proxy\Resolver;

/** @deprecated */
Copy link
Member

Choose a reason for hiding this comment

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

This also lacka information when this will be removed and what to use instead

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, will add documentation around this deprecation as well.

@alcaeus alcaeus requested a review from malarzm May 16, 2021 15:20
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

Thanks!

@alcaeus alcaeus merged commit bf88e32 into doctrine:2.3.x May 16, 2021
@alcaeus alcaeus modified the milestones: 2.3.0-alpha1, 2.3.0 Sep 9, 2021
@alcaeus alcaeus deleted the use-proxy-class-name-resolver branch November 24, 2023 08:10
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