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

CommentFileSizeAuthor
#6 add_change_record_to-2897015-6.patch1.67 KBpk188
#4 add_change_record_to-2897015-3.patch1.2 KBpk188
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Issue summary: View changes
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Title: EntityManager @deprecation is incomplete » EntityManager class/method @deprecation is incomplete
pk188’s picture

pk188 CreditAttribution: pk188 at OpenSense Labs commented
Status: Active » Needs review
FileSize
add_change_record_to-2897015-3.patch1.2 KB

Thanks @Mile23.
I added the patch of issue #2873798 here.

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Status: Needs review » Needs work

So far everything looks good.

Hi pk188,

I added the patch of issue #2873798 here.

If you wrap a bracket and # symbol around the issue number like [#xyz] then nice highlighting will happen.

useCaches() needs a little work.

pk188’s picture

pk188 CreditAttribution: pk188 at OpenSense Labs commented
Status: Needs work » Needs review
FileSize
add_change_record_to-2897015-6.patch1.67 KB

Thanks @martin107.
I updated the patch for useCaches().

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Component: entity system » documentation
Status: Needs review » Reviewed & tested by the community

pk188++

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.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs work

Thanks 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!

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

These are not the methods @Mile23 was referring to

Actually, 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 implements EntityManagerInterface. It says, in its docblock:

Deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. We cannot use the deprecated PHPDoc tag because this service class is still used in legacy code paths. Symfony would fail test cases with deprecation warnings.

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.'

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Issue summary: View changes
Related issues: +#2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED, +#2782833: Remove usages of deprecated EntityManager from Entity class

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Parent issue: » #2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED
Related issues: -#2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Closed (duplicate)

I think this has or is being covered all in #2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED now.