Problem/Motivation

Now we use the construction: dirname(dirname(dirname(dirname(__DIR__)))). Much better and nicer is to have: dirname(__DIR__, 4).

Proposed resolution

Change multiple dirname()'s into a single dirname() with the use of the second parameter of dirname().

Now we have something like: dirname(dirname(dirname(dirname(__DIR__)))).

It is better and nicer to have it change to: dirname(__DIR__, 4).

Remaining tasks

TBD

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

None

CommentFileSizeAuthor
#6 interdiff.txt3.32 KBLal_
#6 3129508-5.patch20.36 KBLal_
#2 3129508-2.patch18.15 KBLal_

Comments

daffie created an issue. See original summary.

Lal_’s picture

Status: Active » Needs review
StatusFileSize
new18.15 KB
daffie’s picture

Status: Needs review » Needs work

@Lal_ Did you use the 9.1.x branch for your patch and did you do a "git pull" before you start making your patch?

daffie’s picture

The changes in the patch look good. Just one nitpick:

+++ b/core/lib/Drupal/Core/Test/PhpUnitTestRunner.php
@@ -91,7 +91,7 @@ public function phpUnitCommand() {
+    $vendor_dir = dirname($reflector->getFileName(),2);

Please add a space between the comma and to number 2.

suresh prabhu parkala’s picture

Status: Needs work » Needs review
StatusFileSize
new18.08 KB

There was an error while applying the patch for 9.1.x. Here is the updated patch. Please review!

Lal_’s picture

StatusFileSize
new20.36 KB
new3.32 KB
Lal_’s picture

@suresh I missed few places adding the interdiff wrt to #2 and #6

daffie’s picture

Status: Needs review » Reviewed & tested by the community

All code changes look good.
Tested that all double or more usages are changed.
For me it is RTBC.

  • catch committed 8f7f02c on 9.1.x
    Issue #3129508 by Lal_, daffie, Suresh Prabhu Parkala: Change multiple...

  • catch committed 3edf9bf on 9.0.x
    Issue #3129508 by Lal_, daffie, Suresh Prabhu Parkala: Change multiple...
catch’s picture

Version: 9.1.x-dev » 9.0.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.1.x and 9.0.x, thanks!

Does not apply to 8.9.x, since it's functionally the same I think it's OK if we don't backport, but it could go into 8.9.x during beta if people really want to.

Status: Fixed » Closed (fixed)

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