-
-
Notifications
You must be signed in to change notification settings - Fork 294
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
Doctrine\Common\Util\ClassUtils being deprecated #867
Comments
@Ocramius You said "If you want, you can make ClassUtils::getRealClass() proxy through to the new API in 3.x, and then we can drop it in 4.x." Does that mean making it not deprecated in 2.x, adding a proxy for it in 3.x (which will be deprecated from the start) and then drop it in 4.x ? If that is so, that is exactly the kind of change I wanted. |
This could be a viable plan:
* in 2.x, that API is deprecated (we still don't want people to do
`get_class()` on proxies), and we remove the warning
* In 3.x, we change the implementation to be compatible with the new proxy
naming, we deprecate it and we add a warning
* In 4.x, we drop that API
…On Wed, 13 Mar 2019, 23:45 Terje Bråten, ***@***.***> wrote:
@Ocramius <https://github.com/Ocramius> You said "If you want, you can
make ClassUtils::getRealClass() proxy through to the new API in 3.x, and
then we can drop it in 4.x."
Does that mean making it not deprecated in 2.x, adding a proxy for it in
3.x (which will be deprecated from the start) and then drop it in 4.x ?
If that is so, that is exactly the kind of change I wanted.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#867 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakDj7KYxSfUQryE6NY3eKcqc_xy0-ks5vWX-KgaJpZM4bxVSu>
.
|
"is deprecated" and "remove the warning" sounds like an oxymoron to me. But as long as we agree that the deprecation warning should go in 2.x, I guess we are on the same page. |
We assembled this document *after* picking a losing fight with
`trigger_error()|, so we can change it:
https://www.doctrine-project.org/policies/deprecation.html
…On Thu, 14 Mar 2019, 00:07 Terje Bråten, ***@***.***> wrote:
"is deprecated" and "remove the warning" sounds like an oxymoron to me.
But as long as we agree that the deprecation warning should go in 2.x, I
guess we are on the same page.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#867 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakFqRsYP5yS7H73Mj7R3csVkd-Bxsks5vWYSzgaJpZM4bxVSu>
.
|
The annotation My suggestion is that we remove the Does that sound ok? |
The annotation has no runtime effects, and the intention for this API to be
removed is still there, which is what we'd communicate with `@deprecated`.
…On Thu, 14 Mar 2019, 00:29 Terje Bråten, ***@***.***> wrote:
Doctrine\Common\Util\ClassUtils starts with
/**
* Class and reflection related functionality for objects that
* might or not be proxy objects at the moment.
*
* @author Benjamin Eberlei ***@***.***>
* @author Johannes Schmitt ***@***.***>
*
* @deprecated The ClassUtils class is deprecated.
*/
class ClassUtils
{
The annotation @deprecated triggers warnings in f.ex. PHPStorm and other
smart editors.
My suggestion is that we remove the @deprecated annotation in 2.x. You
may still have a comment in header that this class is/will be deprecated,
but without the annotation.
Does that sound ok?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#867 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakGob33v2uuvnBb_hGJiehh7rtkiaks5vWYnMgaJpZM4bxVSu>
.
|
As I said, the annotation has effects in my programming environment. When I see something marked as deprecated, I always want to replace it. But it does not make sense to mark it as deprecated, when it is no sensible action the application programmer can do to replace it. |
The correct replacement is
`EntityManager#gerClassMetadata(get_class($proxy))->getName()`, now that I
think of it. Want to document that? (Works also with the
`ClassMetadataFactory` interface)
…On Thu, 14 Mar 2019, 00:35 Terje Bråten, ***@***.***> wrote:
As I said, the annotation has effects in my programming environment.
When I see something marked as deprecated, I always want to replace it.
But it does not make sense to mark it as deprecated, when it is no sensible
action the application programmer can do to replace it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#867 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakEJRlG-SSAM9VgTR_De3ABuJ9sdOks5vWYsmgaJpZM4bxVSu>
.
|
So, my original problem is that I have an object that may or may not be a proxy. So, you are saying that the correct way to do it will be:
|
Yes, if it is something that doctrine knows about
…On Thu, 14 Mar 2019, 00:57 Terje Bråten, ***@***.***> wrote:
So, my original problem is that I have an object that may or may not be a
proxy.
First we tried get_class($object) but that failed when the object was
proxy instead of the real object.
Then we changed it to Doctrine\Common\Util\ClassUtils::getClass($object).
So, you are saying that the correct way to do it will be:
$em = $this->getDoctrineEntityManager();
$classname = $em->getClassMetadata(get_class($object))->getName();
```
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#867 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakP-itXfzmTzxGejxqO6St0EKYYXLks5vWZB-gaJpZM4bxVSu>
.
|
That is a bit complicated replacement. I guess I will need to make my own Util function to contain that then. Yes, I guess it would help if that was documented in the class where the @deprecated annotation is. |
Could you send a PR for that, and maybe also to the upgrade notes?
…On Thu, 14 Mar 2019, 01:01 Terje Bråten, ***@***.***> wrote:
That is a bit complicated replacement. I guess I will need to make my own
Util function to contain that then.
Yes, I guess it would help if that was documented in the class where the
@deprecated <https://github.com/deprecated> annotation is.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#867 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakM2UFjrQgtu9wnV_DkbRBZqWSiuhks5vWZFegaJpZM4bxVSu>
.
|
Ok, have to go now. May be tomorrow or over the weekend. |
Could it also be a new method in the EntityManager? |
Unlikely: the class metadata API is what this is for, and adding methods to
existing classes usually leads to more bloat and BC breaks (for inheritance)
…On Thu, 14 Mar 2019, 01:11 Terje Bråten, ***@***.***> wrote:
Could it also be a new method in the EntityManager?
May be something like $em->getClassName($object);
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#867 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakCsy_J7_eX53PNnsi3e2J0kJb9_hks5vWZOYgaJpZM4bxVSu>
.
|
AFAICT, you cannot implement your plan of making the same API available for 3.x: the proxy classname logic is configurable in ProxyManager, so you will need the EntityManager to get the actual configuration, which a static method cannot do. So we should suggest the metadata-based call as a replacement instead, for people to migrate. |
@stof for the 99% of scenarios, assuming the default inflector will be sufficient. For an endpoint that is going to be deprecated, this is acceptable IMO. |
There's still a chance it'll break for folks using it in 3.x though |
Yes, that is acceptable though, IMO. Can also document it to explain that. |
Is it actually safe to test for Proxy interface (doctrine common) and then |
That's a valid solution, but the proxy interface changes in ORM 3.x and ODM 2.x |
In my application I am using the latest doctrine 2.x. Recently we found it necessary to also use the static function
Doctrine\Common\Util\ClassUtils::getRealClass(string $class)
. But this function is marked as deprecated.I asked in issue 826 what would be the alternative function to use. Then I got the response that there is no alternative to that function in doctrine 2.x.
This issue is about removing those depreaction notices, since I see no point in marking something as deprecated when the application programmers have nothing to replace it with.
See also comment #826 (comment)
The text was updated successfully, but these errors were encountered: