Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables

File /core/modules/system/lib/Drupal/system/Plugin/Block/SystemMenuBlock.php

Line 31: Unused local variable $plugin
Line 39: Unused local variable $plugin

CommentFileSizeAuthor
#1 core-remove-unused-local-variables-systemmenyblock.php-2060721-1.patch1.1 KBlegolasbo

Comments

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
StatusFileSize
new core-remove-unused-local-variables-systemmenyblock.php-2060721-1.patch1.1 KB

Attached patch removes the unused variables.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Active » Needs review

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

The last submitted patch, core-remove-unused-local-variables-systemmenyblock.php-2060721-1.patch, failed testing.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Needs work » Needs review
Issue tags: +Novice

#1: core-remove-unused-local-variables-systemmenyblock.php-2060721-1.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Check that variables ...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs review

Hm. Not sure about this one. It seems to make the code less readable.

michaelhiiva’s picture

michaelhiiva commented
Status: Needs review » Reviewed & tested by the community

The patch applies cleanly, list function on line 32 and 40 can be use this way in PHP and the code is readable.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Yep. Committed/pushed to 8.x, thanks!

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Hmm...
I would have thought the standard is

list(, $foo)

instead of

list( , $foo)

Should we start a docs issue for that?

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