In EntityListController we have:

  public function buildHeader() {
    $row['label'] = t('Label');
    $row['operations'] = t('Operations');
    return $row;
  }

but in VocabularyListController we have:

  /**
   * {@inheritdoc}
   */
  public function buildHeader() {
    $header['label'] = t('Vocabulary name');
    $header['weight'] = t('Weight');
    return $header + parent::buildHeader();
  }

Using a different variable name makes it harder to follow the method inheritance, and also means that it's harder to move code up and down between inherited methods.

I would suggest $header is better than $row as it's more expressive.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

littledynamo’s picture

Assigned: Unassigned » littledynamo

Code sprint planned next week, this task looks ideal for new contributors.

littledynamo’s picture

beowulf1416’s picture

Status: Active » Needs review
FileSize
591 bytes

renamed variable $row to $header in EntityListController::buildHeader()

littledynamo’s picture

@beowulf1416 I'd earmarked this issue for our code sprint next week but didnt realise you were already working in it. Maybe assign to yourself so I know to steer clear :)

littledynamo’s picture

Removed CodeSprintDundee tag

littledynamo’s picture

Assigned: littledynamo » Unassigned

Unassigning

littledynamo’s picture

Assigned: Unassigned » littledynamo
littledynamo’s picture

Assigned: littledynamo » Unassigned

Urgh, no idea how that happened ...

joachim’s picture

It might be worth checking the other overrides of that method too: https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Entity!EntityList...

beowulf1416’s picture

I did the check of the other overrides (16 as of today).
ViewListController::buildHeader()
is totally different. Didn't touch it.

enhdless’s picture

Patch works great.

joachim’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Nice clean-up.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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