Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables
File /core/modules/simpletest/lib/Drupal/simpletest/Tests/SimpleTestTest.php
Line 58: Unused local variable $conf
Line 183: Unused local variable $i
Comment | File | Size | Author |
---|---|---|---|
#21 | remove-unused-local-variable.patch | 1.83 KB | Bladedu |
#19 | remove-unused-local-variable.patch | 2.22 KB | Bladedu |
#15 | remove-unused-local-variable.patch | 2.22 KB | Bladedu |
#12 | 1952062-translation-module-56.patch | 69.45 KB | pfrenssen |
#11 | remove-unused-local-variable.patch | 2.22 KB | Bladedu |
Comments
Comment #1
mrsinguyen CreditAttribution: mrsinguyen commentedComment #3
lokapujyaThe test is purposely generating a divide by zero here.
I removed the assignment; Seems weird, but should work.
Comment #4
mrsinguyen CreditAttribution: mrsinguyen commentedLook good.
Comment #5
catchI think we should leave the $i = 1/ 0 in. It's redundant but it looks really weird not to have it.
Comment #6
lokapujyaCan we generate the error another way or USE the variable so that we accomplish the goal of getting rid of the unused variable?
Comment #7
BladeduI changed the way the warning error is triggered by using trigger_error() function without passing any argument. This should get rid of the unused variable.
Comment #8
lokapujyaThis works for me. I'll let a 2nd eye RTBC it.
Comment #9
lokapujyaOn second thought, I think the error message is a required parameter to trigger_error().
Comment #10
BladeduThe trigger_error requires at least one parameter. Omitting it I was able to trigger the E_WARNING error. Unfortunately trigger_error accepts only E_USER_* types of errors, so it can't be used properly. I will update the patch with some documentation.
Comment #11
BladeduRe-rolled the patch in comment #7 with some comment on how trigger_error() function has been used.
Comment #12
pfrenssenAdding the comment clarifies the usage. Funny that you are triggering an error inside of trigger_error() :)
If the test comes back green this is RTBC for me.
Comment #13
pfrenssenIgnore the patch from #12, I must have had too many beers. The right patch is #11.
Comment #14
alexpottPatch no longer applies.
Comment #15
BladeduRerolled patch #11.
Comment #16
lokapujyaComment contains an extra /
Not a big deal, but can the comments be condensed to 2 lines?
Comment #17
BladeduYou're absolutely right! I made a mistake while rerolling the patch and I haven't noticed it. I'll fix the patch and post it back.
Comment #18
tim.plunkettRemoving tags
Comment #19
BladeduRemoved the extra "/".
Comments cannot be condensed in two lines because it will overflow 80 character length as specified in the Drupal coding standards document.
Comment #21
BladeduLet's try one more time...
Comment #22
lokapujyaThanks for condensing the comment.
Comment #23
webchickCommitted and pushed to 8.x. Thanks!