See meta #2002650: [meta] 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)
Files: 
CommentFileSizeAuthor
#10 8.x-unused-vars-2002732-10.patch1.35 KBdrupalmonkey
PASSED: [[SimpleTest]]: [MySQL] 55,820 pass(es).
[ View ]
#3 2002732-1-performance-unused-vars.patch1.27 KBdiarmy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2002732-1-performance-unused-vars.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 8.x-remove-unused-local-variables-2002732-2.patch1.34 KBFroelund
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 8.x-remove-unused-local-variables-2002732-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Assigned:Unassigned» diarmy

StatusFileSize
new1.34 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 8.x-remove-unused-local-variables-2002732-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Sorry, didn't see it was already assigned.

Status:Active» Needs review
StatusFileSize
new1.27 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2002732-1-performance-unused-vars.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
  • Removed $method_id key from foreach statement on line 441
  • Removed $conf on line 630
  • Removed $default_data on line 1509

Sorry 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

Removing blank line looks better for me (#2).
I'd also rather remove $default_data as in #3.

So, combined effort would do it.

I 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.

How about removing comment then? Things become obvious with that variable.

Good idea, I'll supply a new patch.

Status:Needs review» Needs work

The last submitted patch, 2002732-1-performance-unused-vars.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.35 KB
PASSED: [[SimpleTest]]: [MySQL] 55,820 pass(es).
[ View ]

I 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.

Looks good! Thanks guys.

Status:Needs review» Needs work
Issue tags:-Novice

The last submitted patch, 8.x-unused-vars-2002732-10.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Novice

#10: 8.x-unused-vars-2002732-10.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

looking good

Title:Improve performance by removing unused local variables - core/includes/update.incImprove code maintainability by removing unused local variables - core/includes/update.inc
Status:Reviewed & tested by the community» Fixed

Committed 0979e1f and pushed to 8.x. Thanks!

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