See meta #2002650: [meta, no patch] improve maintainability by removing unused local variables

core/includes/authorize.inc, line numbers identified below.

  • Unused local variable $base_url (18)
  • Unused local variable $base_url (229)
CommentFileSizeAuthor
#2 2002706-2-remove_unused_variables.patch694 byteskerasai
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kerasai’s picture

Assigned: Unassigned » kerasai
kerasai’s picture

Status: Active » Needs review
FileSize
694 bytes

Patch attached.

YesCT’s picture

Status: Needs review » Reviewed & tested by the community

this looks good. within scope and nothing tricky.

rtbc if testbot says ok.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2002706-2-remove_unused_variables.patch, failed testing.

neochief’s picture

Dude, please update the task on drupalofficehours if you taken task from there.

kerasai’s picture

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

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

The last submitted patch, 2002706-2-remove_unused_variables.patch, failed testing.

jthorson’s picture

Patch segfaulted apache on the testbot. Without investigating further, I'm not sure if it was the patch or the bot's fault.

kerasai’s picture

Status: Needs work » Needs review
kerasai’s picture

Issue tags: +Needs manual testing

First time through it passed all tests except for Drupal\system\Tests\Form\RedirectTest, second time looks like environment choked on setup. This is a simple edit, and should clearly not cause any errors or changes to the operation of the functions.

Giving it one more shot through the testbot, will also manually conduct tests to see if this edit impacts behavior.

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing

This should be ok :)

alexpott’s picture

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

Fixed title as this is not a big performance win.. but unused variables are a code maintainability issue... 2 less lines of code to maintain!

Committed 6455242 and pushed to 8.x. Thanks!

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