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

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

Line 71: Unused local variable $num_records_before

CommentFileSizeAuthor
#7 drupal8.unused_truncate_var.2080117-7.patch669 bytesEnxebre
#4 drupal8.unused_truncate_var.2080117-4.patch713 bytesmcrittenden

Comments

chertzog’s picture

chertzog
he/him
Primary language English
Location Harrisburg, PA, USA
commented
Status: Active » Closed (duplicate)

Closing as duplicate of #2080311: Clean up Drupal\system\Tests\Database\SelectTest.php

angel.h’s picture

angel.h
Primary language English
Location Plovdiv
commented
Status: Closed (duplicate) » Active
angel.h’s picture

angel.h
Primary language English
Location Plovdiv
commented

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

mcrittenden’s picture

mcrittenden commented
Status: Active » Needs review
StatusFileSize
new drupal8.unused_truncate_var.2080117-4.patch713 bytes

This one just removes that line altogether since it's not being used anywhere.

enhdless’s picture

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

Patch works great.

webchick’s picture

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

Hm. I don't think this is quite right, since it changes the nature of the test. Seems like we should instead add an assertion that makes use of $num_records_before.

Enxebre’s picture

Enxebre commented
Status: Needs work » Needs review
StatusFileSize
new drupal8.unused_truncate_var.2080117-7.patch669 bytes

Adding assertion checking $num_records_before is greater than 0

areke’s picture

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

The patch applies and looks good to me.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Closed (duplicate)
Related issues: +#2080343: Remove Unused local variables from system module

Please combine this patch with #2080343: Remove Unused local variables from system module.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Remove Unused local variable $num_records_before from lib/Drupal/system/Tests/Database/DeleteTruncateTest.php » Add assertion to Drupal/system/Tests/Database/DeleteTruncateTest.php
Component: other » database system
Status: Closed (duplicate) » Reviewed & tested by the community

Reopening and retitling to reflect what the patch is doing. Thanks!

webchick’s picture

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

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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