Part of meta-issue #2002650: [meta] 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

Files: 
CommentFileSizeAuthor
#15 drupal-remove-unused-var-2062213-15.patch572 bytesduozersk
PASSED: [[SimpleTest]]: [MySQL] 58,833 pass(es).
[ View ]
#8 drupal-remove-unused-var-2062213-8.patch921 bytesduozersk
PASSED: [[SimpleTest]]: [MySQL] 58,288 pass(es).
[ View ]
#5 drupal-remove-unused-var-2062213-5.patch963 bytesduozersk
PASSED: [[SimpleTest]]: [MySQL] 58,194 pass(es).
[ View ]
#1 drupal-remove-unused-var-2062213-1.patch964 bytesduozersk
PASSED: [[SimpleTest]]: [MySQL] 57,789 pass(es).
[ View ]

Comments

Assigned:duozersk» Unassigned
Status:Active» Needs review
StatusFileSize
new964 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,789 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

nice

Status:Reviewed & tested by the community» Needs work

+++ b/core/lib/Drupal/Core/StreamWrapper/LocalStream.phpundefined
@@ -87,7 +87,7 @@ protected function getTarget($uri = NULL) {
+    list( , $target) = explode('://', $uri, 2);

The list( , should be list(, as there are 14 examples of this in core and only 1 example of the other way around.

Assigned:Unassigned» duozersk

Good, will do.

Status:Needs work» Needs review
StatusFileSize
new963 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,194 pass(es).
[ View ]

Assigned:duozersk» Unassigned
Status:Needs review» Reviewed & tested by the community

RTBC as it was already reviewed.

Status:Reviewed & tested by the community» Needs review

As mentioned elsewhere, these ones that change list($foo, $bar) to list($foo, ) make the code less readable. Is there another way we can remove the error without doing that?

Assigned:Unassigned» duozersk
StatusFileSize
new921 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,288 pass(es).
[ View ]

Not sure if it is the optimal way to do this.
Basically, used the strpos() and drupal_substr().

Assigned:duozersk» Unassigned

Status:Needs review» Reviewed & tested by the community

Suppose this approach is less readable but according @webchick this is another way

Status:Reviewed & tested by the community» Fixed

I committed #5 sometime in my recent commit spree, but apparently lost my comment!

As far as I know, we can simply do list($foo) which is actually much more readable.

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

Status:Fixed» Needs work

In this case I think a follow-up patch would be preferred. Thanks @duozersk!

StatusFileSize
new572 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,833 pass(es).
[ View ]

OK, here it goes. Removed 2 symbols ", " per #12 above.

Assigned:Unassigned» duozersk
Status:Needs work» Needs review

Waiting for the test results.

Status:Needs review» Reviewed & tested by the community

Awesome, thanks! Patch looks great.

Assigned:duozersk» Unassigned

Status:Reviewed & tested by the community» Fixed

Committed 9a7ef5d and pushed to 8.x. Thanks!

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