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

File /core/modules/system/lib/Drupal/system/Tests/Database/SelectOrderedTest.php

Line 30: Unused local variable $name_field
Line 51: Unused local variable $name_field
Line 82: Unused local variable $name_field

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chertzog’s picture

Status: Active » Closed (duplicate)
angel.h’s picture

Status: Closed (duplicate) » Active

This issue shouldn't have been closed - see https://drupal.org/node/2080311#comment-7909573.

pieterjd’s picture

Status: Active » Needs review
FileSize
1.3 KB

Status: Needs review » Needs work

The last submitted patch, drupal8.other_.2080289-3.patch, failed testing.

pieterjd’s picture

Since it is my first patch - I went to the workshop yesterday at DrupalCon Prague - I followed the guide and doublechecked the required removals. I would like to bring this patch to a good end, any suggestions to make this patch pass the test?

herom’s picture

@pieterjd You shouldn't have removed the whole lines; just the declaration part: $name_field = . You should check to make sure the lines you remove don't have any effect in the code.
Here, in the second case, the query is expected to have the name field (see $expected variable there), and the test fails without it.

mcrittenden’s picture

@pieterjd, if you're interested in following through with a patch, it's also useful to assign it to yourself via the "Assigned" field when updating it.

pieterjd’s picture

Assigned: Unassigned » pieterjd
Status: Needs work » Needs review
FileSize
1.42 KB
cosmicdreams’s picture

Issue tags: -Novice

#8: drupal8.other_.2080289-8.patch queued for re-testing.

areke’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

The patch correctly gets rid of the unused variables, and keeps the tests. The patch applies, so it looks good. Thank you!

xjm’s picture

Status: Reviewed & tested by the community » Closed (duplicate)
Related issues: +#2080343: Remove Unused local variables from system module