Remove Unused local variable $base_field from /core/modules/views/lib/Drupal/views/Plugin/views/relationship/GroupwiseMax.php [#2072595]

Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables

File /core/modules/views/lib/Drupal/views/Plugin/views/relationship/GroupwiseMax.php

Line 222: Unused local variable $base_field

Comment	File	Size	Author
#3	core-remove-unused-vars-2072595-3.patch	779 bytes	legolasbo
#3
#1	core-remove-unused-vars-groupwisemax.php-2072595-1.patch	735 bytes	legolasbo
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

legolasbo

he/him

Dutch

Middelburg

CreditAttribution: legolasbo commented 23 August 2013 at 22:12

Assigned:	legolasbo	» Unassigned
Status:	Active	» Needs review

File	Size
core-remove-unused-vars-groupwisemax.php-2072595-1.patch	735 bytes

Removed unused variable

Comment #2

phiit CreditAttribution: phiit commented 29 August 2013 at 12:07

Status:

Needs review

» Needs work

Reviewed, this patch fixes unused variable $base_field but after cleanup leaves another unused variable $views_data which was used only in $base_field.

@@ -219,7 +219,6 @@ protected function leftQuery($options) {
 
     // Add the base table ID field.
     $views_data = Views::viewsData()->get($this->definition['base']);
-    $base_field = $views_data['table']['base']['field'];

Comment #3

legolasbo

he/him

Dutch

Middelburg

CreditAttribution: legolasbo commented 31 August 2013 at 20:46

Status:

Needs work

» Needs review

File	Size
core-remove-unused-vars-2072595-3.patch	779 bytes

Attached patch also removes $views-data.

Comment #4

Anonymous (not verified) CreditAttribution: Anonymous commented 21 September 2013 at 13:58

The patch in #3 still applies OK however I am unsure as to whether $base_field should actually be used or not. Here's the code:

    // Add the base table ID field.
    $views_data = Views::viewsData()->get($this->definition['base']);
    $base_field = $views_data['table']['base']['field'];
    $temp_view->addItem('default', 'field', $this->definition['base'], $this->definition['field']);

...perhaps in this line somewhere:

    $temp_view->addItem('default', 'field', $this->definition['base'], $this->definition['field']);

If not then the patch is good to go as far as I can see.

Comment #5

boran CreditAttribution: boran commented 23 September 2013 at 20:16

Component:	other	» views.module
Status:	Needs review	» Reviewed & tested by the community

#3 is fine:
- makes sense
- patch applies cleanly
- Comment #4: $base_field is not used, although the words base and field do appear :-)
- views continues working