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.
See meta #2002650: [meta, no patch] improve maintainability by removing unused local variables
core/includes/theme.inc
- Unused local variable $themes (line 746)
- Unused local variable $number (line 2860)
- Unused local variable $i (line 2201)
- Unused local variable $number (line 2264)
- Unused local variable $user (line 2674)
Comment | File | Size | Author |
---|---|---|---|
#10 | 8.x-unused-vars-2002730-9.patch | 1.69 KB | drupalmonkey |
#3 | removing-unused-local-variables-2002730.patch | 2.6 KB | leanderl |
#2 | 8.x-remove-unused-local-variables-2002730-2.patch | 1.39 KB | Froelund |
Comments
Comment #1
Froelund CreditAttribution: Froelund commentedWorking on it.
Comment #2
Froelund CreditAttribution: Froelund commentedComment #3
leanderl CreditAttribution: leanderl commentedRemoved the variables -- couldn't find $number on row 2860: "Unused local variable $number (line 2860)". Also checked on row 1860 and 860.
Comment #4
neochief CreditAttribution: neochief commented#2 looks better for me (there's weird code block removal in #3 + no new line removal)
Comment #5
leanderl CreditAttribution: leanderl commentedWhat does "+ no new line removal" mean? Would be nice if you could be more specific. Is there a new line somewhere that should be removed? I would love to learn to be a better patch contributor.
The code block that was removed was as far as I could tell relating to the deleted variable and could/should thus be deleted. But of course I could be wrong.
Comment #6
neochief CreditAttribution: neochief commentedNo new line removal means the line after "- global $user;"
As for the block of code, I think it's not useless. You should have removed just a "$themes = array();" as this line is useless, since the variable will be reassigned later anyway (see the if-else statements).
Comment #8
leanderl CreditAttribution: leanderl commented@neochief, yes it turns out you were right about the code block. It should stay in.
Comment #9
connorwk CreditAttribution: connorwk commented#3: removing-unused-local-variables-2002730.patch queued for re-testing.
Comment #10
drupalmonkey CreditAttribution: drupalmonkey commentedRerolled the patch from #2, removed the other unnecessary $number variable from line ~2180ish.
Comment #11
connorwk CreditAttribution: connorwk commentedThis is removing more then expected.
Comment #12
kerasai CreditAttribution: kerasai commented@connork's comment in #11 is referring to the patch submitted in #2.
#10 looks good, resubmitting to testbot.
Comment #13
kerasai CreditAttribution: kerasai commented#10: 8.x-unused-vars-2002730-9.patch queued for re-testing.
Comment #14
aspilicious CreditAttribution: aspilicious commentedLooking good
Comment #15
alexpottCommitted abbd5be and pushed to 8.x. Thanks!