Problem/Motivation

Drupal\Core\Entity\EntityHandlerBase is to be deprecated in #2471663: Undeprecate EntityHandlerBase, but is not marked as such.

#2471689: Entity API documentation should consistently refer to handlers rather than controllers might guide the way on what to tell people about it.

Proposed resolution

  • Mark it as @deprecated.
  • Figure out what to tell people about it in the @deprecation block.

Remaining tasks

User interface changes

API changes

Data model changes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because we are marking a class as deprecated, which is a docs-only change.
Issue priority Normal because nothing is broken other than our API.
CommentFileSizeAuthor
#19 interdiff-2539848-16-19.txt684 bytespguillard
#19 deprecated-2539848-19.patch716 bytespguillard
#16 interdiff-12-16.txt563 bytessdstyles
#16 2539848-16.patch610 bytessdstyles
#12 interdiff-10-12.txt495 bytessdstyles
#12 2539848-12.patch594 bytessdstyles
#10 interdiff-5-10.txt556 bytessdstyles
#10 interdiff-7-10.txt588 bytessdstyles
#10 2539848-10.patch590 bytessdstyles
#7 deprecated-2539848-6.patch467 bytesankur7213
#5 interdiff-2539848-1-5.txt575 bytesdeepakaryan1988
#5 mark_EntityHandlerBase_as_deprecated-2539848-5.patch603 bytesdeepakaryan1988
#1 mark_EntityHandlerBase_as_deprecated-2539848-1.patch609 bytespguillard

Comments

pguillard’s picture

pguillard commented
Status: Active » Needs review
StatusFileSize
new mark_EntityHandlerBase_as_deprecated-2539848-1.patch609 bytes

I'm not sure I understood.
Just a guess then...

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Still applies.

cilefen’s picture

cilefen commented
Status: Needs review » Needs work
Issue tags: +Novice, +Quick fix, +Documentation 
+++ b/core/lib/Drupal/Core/Entity/EntityHandlerBase.php
@@ -14,7 +14,8 @@
+ * @deprecated in Drupal 8.x, will be removed before Drupal 9.0.
+ * Use \Drupal\core\lib\Drupal\Core\Entity\EntityHandlerInterface instead

The second line must be indented and end with a period. See UrlGeneratorInterface for examples in core.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Coding standards for API docs: https://www.drupal.org/coding-standards/docs#deprecated

deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
StatusFileSize
new mark_EntityHandlerBase_as_deprecated-2539848-5.patch603 bytes
new interdiff-2539848-1-5.txt575 bytes

Lets hope this is correct.

deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
Status: Needs work » Needs review
ankur7213’s picture

ankur7213 commented
StatusFileSize
new deprecated-2539848-6.patch467 bytes

i have attached patch file.

meramo’s picture

meramo commented
Status: Needs review » Needs work

I think it still needs work, as

  • Line 16 is missing the asterisk (blank line instead)
  • @todo statement should be removed
  • Ideally we would like to have a line explaining why it's marked as such
  • If the @todo statement is left, a blank line should be between sections
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented

@meramo I have removed the @todo statement. @ankur7213 Why did you re-rolled this patch?

Please let me know.

sdstyles’s picture

sdstyles commented
Status: Needs work » Needs review
StatusFileSize
new 2539848-10.patch590 bytes
new interdiff-7-10.txt588 bytes
new interdiff-5-10.txt556 bytes

Marked as deprecated EntityHandlerBase and corrected namespace to EntityHandlerInterface.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/lib/Drupal/Core/Entity/EntityHandlerBase.php
@@ -14,7 +14,9 @@
+ * @deprecated in Drupal 8.x, will be removed before Drupal 9.0.

8.0.x, 9.0.0

sdstyles’s picture

sdstyles commented
StatusFileSize
new 2539848-12.patch594 bytes
new interdiff-10-12.txt495 bytes

Fixed.

deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented

@tim.plunkett @Mile23 @cilefen I think #12 works!
What do you say?

meramo’s picture

meramo commented
Status: Needs review » Reviewed & tested by the community

Applies and looks good. Why not to rtbc

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Normally a deprecation notice tells a user what to do instead. Which I think in this case is just inject the module handler dependency using EntityHandlerInterface::createInstance().

sdstyles’s picture

sdstyles commented
Status: Needs work » Needs review
StatusFileSize
new 2539848-16.patch610 bytes
new interdiff-12-16.txt563 bytes

Changed.

meramo’s picture

meramo commented

I think it's good and complies with https://www.drupal.org/node/1876842 and https://www.drupal.org/node/1354#deprecated

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work

I think it should be more specific and say something like: "Implement the container injection pattern of \Drupal\Core\Entity\EntityHandlerInterface::createInstance() to obtain the module handler service for your class."

pguillard’s picture

pguillard commented
Status: Needs work » Needs review
StatusFileSize
new deprecated-2539848-19.patch716 bytes
new interdiff-2539848-16-19.txt684 bytes

#18 applied

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Reviewed & tested by the community

Diggit. Thanks. :-)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 5d3f318 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed 5d3f318 on 8.0.x 
    Issue #2539848 by sdstyles, pguillard, deepakaryan1988, ankur7213: Mark...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

ankur7213’s picture

ankur7213 commented