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/forum/lib/Drupal/forum/Tests/ForumTest.php
Line 271: Unused local variable $mlid
Line 534: function verifyForums includes a $node_user argument which is unused also.
Comment | File | Size | Author |
---|---|---|---|
#17 | forum-unused-crud-2080543.17.patch | 3.84 KB | larowlan |
#17 | interdiff.txt | 412 bytes | larowlan |
Comments
Comment #1
mrsinguyen CreditAttribution: mrsinguyen commentedComment #2
grisendo CreditAttribution: grisendo commentedThe patch attached is not for this issue, is the corresponding to #2080539
I send to re-test because it also can't be applied via git.
I attach the real one.
Comment #3
grisendo CreditAttribution: grisendo commented#1: drupal-core-remove-unused-local-variable-2080539.patch queued for re-testing.
Comment #4
angel.hComment #5
angel.hThe patch looks fine but I get 2 more unused variables in this file when running PHPMD:
core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php:528 Avoid unused parameters such as '$node_user'.
core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php:629 Avoid unused parameters such as '$forum'.
They are not such performance trouble but still should be included in this ticket.
Comment #6
angel.hHere is the new patch and interdiff.
Comment #7
royal121 CreditAttribution: royal121 commentedThe latest patch applies cleanly. Thanks.
Comment #8
areke CreditAttribution: areke commented+1 from me as well. This looks good... RTBC
Comment #9
webchickHm. That's quite a significant change. Sending to larowlan (Forum module maintainer) for review.
Comment #10
xjmLet's also check the rest of the module for unused local variables.
Comment #11
parthipanramesh CreditAttribution: parthipanramesh commentedLooks good!
Comment #12
xjm6: drupal-core-remove-unused-local-variable-2080543-4.patch queued for re-testing.
Comment #13
alexpottThe changes look fine to me to but @webchick has asked for @larowlan's input and the issue summary could do with an update.
Comment #14
larowlanYes, that parameter is unused as well.
Comment #15
larowlanThere are three unused use statements in forum.module - should they be removed here too?
these three:
Comment #16
alexpottAs we're cleaning unused things - yep lets remove the unused use statements too :)
Comment #17
larowlanplease don't credit me for this
Comment #18
pfrenssenLooking good!
Comment #19
webchickToo late. :P
Committed and pushed to 8.x. Thanks!