_comment_get_comment_fields() is weird, super costly and unused

- weird: fetches a list of Field config entities, and force-adds a ->bundles property on them - doesn't seem kosher
- super costly: entity_load_multiple_by_properties('field_entity') means loading all field definitions,
then *for each resulting field*, entity_load_multiple_by_properties('field_instance') means loading all instance definitions.
- unused: it's never called currently ? Plus, its phpdoc mentions it as a "maintenance mode" replacement for comment_get_comment_fields(), which itself doesn't exist...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Let's see what breaks if we remove it ?

yched’s picture

Status: Active » Needs review
longwave’s picture

Status: Needs review » Reviewed & tested by the community

Looked at the commit where this was first introduced, it wasn't called then either, so seems like this can be safely removed.

larowlan’s picture

Yeah it was moved to the manager.
We should look at the performance of that too.

andypost’s picture

Nice catch! Implementation in manager is ok

class CommentManager implements CommentManagerInterface {
  public function getAllFields() {
    $map = $this->fieldInfo->getFieldMap();
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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