Problem/Motivation
#2873798: Add Change record to @deprecated for EntityManager and #2867882: Expand @deprecated docblocks in EntityManager.php fill out the deprecation information for EntityManager
methods which are marked as @deprecated
.
However, there are some EntityManager
methods which are not marked as deprecated, and were in fact not deprecated during #2337191: Split up EntityManager into many services
They are:
getExtraFields()
onEntityTypeCreate()
useCaches()
They are mentioned as deprecated or otherwise changed in the change record: https://www.drupal.org/node/2549139
Proposed resolution
Add a @deprecated and @see tags to the class docblock. Do not throw E_USER_DEPRECATED until after we remove usages: #2782833: Remove usages of deprecated EntityManager from Entity class #2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED
Mark the remaining methods as deprecated.
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#6 | add_change_record_to-2897015-6.patch | 1.67 KB | pk188 |
#4 | add_change_record_to-2897015-3.patch | 1.2 KB | pk188 |
Comments
Comment #2
Mile23Comment #3
Mile23Comment #4
pk188 CreditAttribution: pk188 at OpenSense Labs commentedThanks @Mile23.
I added the patch of issue #2873798 here.
Comment #5
martin107 CreditAttribution: martin107 as a volunteer commentedSo far everything looks good.
Hi pk188,
If you wrap a bracket and # symbol around the issue number like [#xyz] then nice highlighting will happen.
useCaches() needs a little work.
Comment #6
pk188 CreditAttribution: pk188 at OpenSense Labs commentedThanks @martin107.
I updated the patch for useCaches().
Comment #7
martin107 CreditAttribution: martin107 as a volunteer commentedpk188++
A documentation only patch ... it is not wrong to describe this as a entity_system component, but if we relabel as documentation then a wider range of commiters can act on this patch
It all looks good to me.
Comment #8
xjmThanks for working on this issue!
These are not the methods @Mile23 was referring to; they are implementations of them. The deprecations, if appropriate, should go on the corresponding interface(s) instead. (In general, for overridden methods, the docblock should contain
@{inheritdoc}
and nothing else.)So, let's look for the corresponding interface and move this deprecation documentation there. Thanks!
Comment #9
Mile23Actually, they are.
EntityManagerInterface
is marked as deprecated. It has has two methods in it, and both are marked deprecated.EntityManager
is the only class that implementsEntityManagerInterface
. It says, in its docblock:This is incorrect, by the way. phpunit-bridge will only fail tests if we have it throw E_USER_DEPRECATED, which is in #2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED
The change record for the deprecation mentions that the remaining un-deprecated methods have been replaced by other services.
That's why the issue is titled 'EntityManager @deprecation is incomplete.'
Comment #10
Mile23Comment #14
andypostComment #16
BerdirI think this has or is being covered all in #2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED now.