Deprecate doctrine/cache in favor of psr/cache

[derrabus]

Q	A
Type	improvement
BC Break	no
Fixed issues	N/A

Summary

Follows #4620 by adding triggers for deprecation warnings for the old Doctrine Cache related methods.

[greg0ire]

To be retargeted to 3.3.x once 3.2.0 is released.

[derrabus]

This should target 3.3.x. 🙂

[greg0ire]

IMO we should not create 3.3.x before 3.2.0 is released: that would make the 3-steps-merges-up @morozov performs on a regular basis 4-steps-merges-up. Converting this to draft will prevent merges, and adding it to milestone 3.3.0 should prevent us from forgetting it when the time comes.

[derrabus]

All right. I've converted its follow-up #4626 to a draft as well.

[morozov]

Why do we need another minor release to deprecate the old cache?

[greg0ire]

So that calling (Doctrine, but not necessarily) projects can use the new API and have their callers not experience deprecations these callers cannot fix.

[morozov]

So that calling (Doctrine, but not necessarily) projects can use the new API and have their callers not experience deprecations these callers cannot fix.

This is the thing I never understood about runtime deprecations. The API is deprecated. As a consumer, you should be able to disable them at runtime, it shouldn't require another release to support two ways to communicate the deprecation.

[alcaeus]

We can also deprecate them in the same minor release - we've taken this strategy to defer announcing deprecations using a deprecation library to a later release if the deprecation is in the hot path to avoid hundreds of deprecation warnings. In this case, this is even more important because in many cases, the user is not the one calling these methods (e.g., the Symfony bundle configures these calls).

[greg0ire]

Yes, IMO the best DX is when you have no deprecation to disable because you can act on all of them immediately. In our case, we know we need to address these deprecations in our dependent packages, so let's use that knowledge and let our users know about the deprecations when we have addressed them ourselves.

[morozov]

if the deprecation is in the hot path to avoid hundreds of deprecation warnings

I believe this argument is irrelevant given that the current implementation triggers once per call.

have addressed them ourselves

This way, we're introducing coupling of a lower-level entity (cache library) to a higher-level one (its consumer, regardless of whether it's maintained within the same org).

It might make sense if the deprecation library only triggered the deprecation if a deprecated API was used directly from within the application code, not through a dependency.

[greg0ire]

Oh right, I just read the docs and indeed, there is de-duplication based on the link. @derrabus sorry, but I think in this case, you can ignore my comment. It comes from a situation with doctrine/persistence where we used @trigger_error but with doctrine/deprecations, things should be less noisy indeed.

[morozov]

So why do we need to need to have another minor release to release these deprecations? The whole point of using runtime deprecations is to make them visible to the user. If there is a case when the existing deprecations would be false positives, it should be fixed at the deprecation layer, not by introducing a two-phase deprecation process.

[greg0ire]

I changed the milestone to 3.2.0, I thought that would make it clear that I no longer think we need another minor release, but if you want me to argue that point again for the pleasure of having an argument, I can give it another try 😛

The whole point of using runtime deprecations is to make them visible to the user.

One could say here "visible to the user if and only if they can act on it by adapting their code".

If there is a case when the existing deprecations would be false positives, it should be fixed at the deprecation layer, not by introducing a two-phase deprecation process.

Not really false positives, because unfixable deprecations are still legitimate, but are sent to the wrong recipient. To fix that in the deprecation layer, we could have a backtrace analysis that detects where the calling code resides: in the root package or in another package, much like what I've done in the phpunit-bridge, and silence that. Fair warning though: it's complex code that is hard to test, with many edge-cases to handle (code from stdin, code from phars, non standard vendor directories, deprecations triggered when parsing a config file owned by the user).

[derrabus]

I'll adjust the PR for 3.2 then. 🙂

[morozov]

I changed the milestone to 3.2.0, I thought that would make it clear that I no longer think we need another minor release

I apologize. I did notice the change in the milestone but I didn't realize that this is the next minor.

One could say here "visible to the user if and only if they can act on it by adapting their code".

Agree. This is why I believe the deprecation library could use an improvement.

To fix that in the deprecation layer, we could have a backtrace analysis that detects where the calling code resides

Correct, and the library already does that to determine whether the call comes from within the same library that triggers the deprecation.

it's complex code that is hard to test, with many edge-cases to handle (code from stdin, code from phars, non standard vendor directories, deprecations triggered when parsing a config file owned by the user)

While it's true, it shouldn't be used as a reason to not improve the library. The library must act as a no-op in production. In the development environment, the risks of the library misbehaving should be accepted by developers. In the end, it's there for their own good. Otherwise, they can always disable the reporting (which is disabled by default).

[greg0ire]

I guess I'll open yet another RFC then 🙂

[derrabus]

PR is ready again.

[greg0ire]

Thanks @derrabus !