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

File /core/modules/views/lib/Drupal/views/Tests/Handler/FieldWebTest.php

Line 207: Unused local variable $base
Line 208: Unused local variable $absolute_string
Line 232: Unused local variable $expected_result
Line 234: Unused local variable $result

CommentFileSizeAuthor
#4 drupal-core_remove-unused-variable-2072617-4.patch698 bytesdeneo
#1 core-remove-unused-vars-fieldwebtest.php-2072617-1.patch698 byteslegolasbo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo commented
FileSize
core-remove-unused-vars-fieldwebtest.php-2072617-1.patch698 bytes

Removed unused variables, but left $expected_result and $result untouched even though they are currently unused. The line that uses them has been commented out because of a bug in views.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo commented
Assigned: legolasbo » Unassigned
Status: Active » Needs review
areke’s picture

areke CreditAttribution: areke commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll

Unfortunately, the patch no longer applies.

deneo’s picture

deneo CreditAttribution: deneo commented
FileSize
drupal-core_remove-unused-variable-2072617-4.patch698 bytes

I didn't remove $expected_result and $result because in the comments says that aren't used for now because of a bug.

deneo’s picture

deneo CreditAttribution: deneo commented
Status: Needs work » Needs review
areke’s picture

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

Not removing the two local variables was the way to go. The patch applies cleanly and addresses the issue correctly. Thank you!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work

Seems like the comment needs to be removed/updated as well?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Closed (duplicate)

Please merge this (and any other similar cleanups for the Views module) with #2072597: Remove Unused local variables from tests in the Views module.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Related issues: +#2072597: Remove Unused local variables from tests in the Views module