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/update.inc
- Unused local variable $method_id (line 441)
- Unused local variable $conf (line 630)
- Unused local variable $default_data (line 1509)
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 8.x-unused-vars-2002732-10.patch | 1.35 KB | drupalmonkey |
| #3 | 2002732-1-performance-unused-vars.patch | 1.27 KB | diarmy |
| #2 | 8.x-remove-unused-local-variables-2002732-2.patch | 1.34 KB | Froelund |
Comments
Comment #1
diarmy CreditAttribution: diarmy commentedComment #2
Froelund CreditAttribution: Froelund commentedSorry, didn't see it was already assigned.
Comment #3
diarmy CreditAttribution: diarmy commentedComment #4
diarmy CreditAttribution: diarmy commentedSorry about duplicating your effort, Froelund; I didn't refresh my browser before uploading the patch. If you would like to review my patch then we can resolve the confusion.
See reviewing patches on drupal.org
Comment #5
neochief CreditAttribution: neochief commentedRemoving blank line looks better for me (#2).
I'd also rather remove $default_data as in #3.
So, combined effort would do it.
Comment #6
Froelund CreditAttribution: Froelund commentedI thought about removing the variable $default_data, and just read the data from the file directly , but I think it is better for readability to "name" what we are applying, by naming it as the variable $default_data instead.
Comment #7
neochief CreditAttribution: neochief commentedHow about removing comment then? Things become obvious with that variable.
Comment #8
Froelund CreditAttribution: Froelund commentedGood idea, I'll supply a new patch.
Comment #10
drupalmonkey CreditAttribution: drupalmonkey commentedI disagree with #6, the point of these patches is to get rid of unnecessary and unused variables. There is no need to create that variable and unnecessarily use extra memory IMO. Here is a patch that combines the goodness of #2 and #3.
Comment #11
neochief CreditAttribution: neochief commentedLooks good! Thanks guys.
Comment #13
diarmy CreditAttribution: diarmy commented#10: 8.x-unused-vars-2002732-10.patch queued for re-testing.
Comment #14
aspilicious CreditAttribution: aspilicious commentedlooking good
Comment #15
alexpottCommitted 0979e1f and pushed to 8.x. Thanks!