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.
Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables
File /core/lib/Drupal/Core/StreamWrapper/LocalStream.php
Line 90: Unused local variable $scheme
Line 406: Unused local variable $target
Comment | File | Size | Author |
---|---|---|---|
#15 | drupal-remove-unused-var-2062213-15.patch | 572 bytes | duozersk |
#8 | drupal-remove-unused-var-2062213-8.patch | 921 bytes | duozersk |
#5 | drupal-remove-unused-var-2062213-5.patch | 963 bytes | duozersk |
#1 | drupal-remove-unused-var-2062213-1.patch | 964 bytes | duozersk |
Comments
Comment #1
duozerskComment #2
andypostnice
Comment #3
alexpottThe
list( ,
should belist(,
as there are 14 examples of this in core and only 1 example of the other way around.Comment #4
duozerskGood, will do.
Comment #5
duozerskComment #6
duozerskRTBC as it was already reviewed.
Comment #7
webchickAs mentioned elsewhere, these ones that change
list($foo, $bar)
tolist($foo, )
make the code less readable. Is there another way we can remove the error without doing that?Comment #8
duozerskNot sure if it is the optimal way to do this.
Basically, used the
strpos()
anddrupal_substr()
.Comment #9
duozerskComment #10
andypostSuppose this approach is less readable but according @webchick this is another way
Comment #11
webchickI committed #5 sometime in my recent commit spree, but apparently lost my comment!
Comment #12
tstoecklerAs far as I know, we can simply do
list($foo)
which is actually much more readable.Comment #13
duozerskDo we need a follow-up patch then? Should it be just added here or we need to create a new issue? I can do that, just need a guidance here.
Comment #14
tstoecklerIn this case I think a follow-up patch would be preferred. Thanks @duozersk!
Comment #15
duozerskOK, here it goes. Removed 2 symbols ", " per #12 above.
Comment #16
duozerskWaiting for the test results.
Comment #17
tstoecklerAwesome, thanks! Patch looks great.
Comment #18
duozerskComment #19
alexpottCommitted 9a7ef5d and pushed to 8.x. Thanks!