Problem/Motivation

From #2541206: Consider field storage dependency removal on Index (#25.3):

It would also be great not to hard-code :entity: as the separator for an entity reference – even though that's in Core like this, it's still possible to have a reference to an entity in some other way.

Check ContentEntity::getFieldDependenciesForEntityType() (line 899 and 901).

CommentFileSizeAuthor
#5 2656916-4--getFieldDependenciesForEntityType.patch7.85 KBdrunken monkey

Comments

mbovan created an issue. See original summary.

mbovan’s picture

mbovan commented
Issue summary: View changes
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Issue tags: +Release blocker
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review

Er, do we even need that method (ContentEntity::getFieldDependenciesForEntityType())? Seems that's called exactly nowhere. Apparently, we just left it in by accident after taking a different approach in #2541206: Consider field storage dependency removal on Index.
Removing the method. Also, updating the tests to be more thorough regarding field dependencies – I already had done that when I figured out that the method isn't actually called anywhere.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Title: Replace :entity: as a separator for an entity reference » Overhaul or remove ContentEntity::getFieldDependenciesForEntityType()
StatusFileSize
new 2656916-4--getFieldDependenciesForEntityType.patch7.85 KB
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

Removing code AND adding coverage? Is this what a perfect patch looks like?

Testbot isn't back yet but I'm going to rtbc anyway.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Does sound like a good patch, yes. Thanks for reviewing!
And at least one patch without CI error – even better! Committed.

  • drunken monkey committed 6a3a955 on 8.x-1.x 
    Issue #2656916 by drunken monkey: Removed unused field dependency...

Status: Fixed » Closed (fixed)

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