UrlGenerator::setBasePath(), UrlGenerator::setBaseUrl(), and UrlGenerator::setScriptPath() are all redundant and we should remove UrlGenerator::updateFromRequest()

CommentFileSizeAuthor
#10 2343607.10.patch16.49 KBalexpott
#10 6-10-interdiff.txt2.12 KBalexpott
#6 5-6-interdiff.txt2.57 KBalexpott
#6 2343607.6.patch14.82 KBalexpott
#5 interdiff.txt3.35 KBdawehner
#5 2343607-5.patch12.24 KBdawehner
#3 2343607.3.patch9.95 KBalexpott
#3 1-3-interdiff.txt1.47 KBalexpott
#1 2343607.1.patch10.09 KBalexpott

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2343607.1.patch10.09 KB

Status: Needs review » Needs work

The last submitted patch, 1: 2343607.1.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 1-3-interdiff.txt1.47 KB
new 2343607.3.patch9.95 KB

Fixed failing tests

Status: Needs review » Needs work

The last submitted patch, 3: 2343607.3.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2343607-5.patch12.24 KB
new interdiff.txt3.35 KB

Here are the test fixes, took me a little while, though.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 2343607.6.patch14.82 KB
new 5-6-interdiff.txt2.57 KB

Fixed installer tests

dawehner’s picture

dawehner
Primary language German
commented

Just a quick nitpick!

+++ b/core/lib/Drupal/Core/Routing/NullGenerator.php
@@ -19,17 +20,8 @@ class NullGenerator extends UrlGenerator {
   /**
    * Override the parent constructor.
    */
...
+  public function __construct(RequestStack $request_stack) {
+    $this->requestStack = $request_stack;
   }

Would be great to just keep proper docs.

The last submitted patch, 5: 2343607-5.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 6: 2343607.6.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 6-10-interdiff.txt2.12 KB
new 2343607.10.patch16.49 KB

Fixing the test - which is actually broken atm - this does not break it anymore than it already is see #2265099: Cleanup SessionHttpsTest and fix redirect to non-existing URL after POST requests

catch’s picture

catch
he/him
Primary language English
commented

Looks great.

dawehner’s picture

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

+1

catch’s picture

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

Committed/pushed to 8.0.x, thanks!

  • catch committed 03aa670 on 8.0.x 
    Issue #2343607 by alexpott, dawehner: Cleanup UrlGenerator.

Status: Fixed » Closed (fixed)

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