Child of #2417827: Evaluate and document each use of base: in core. See that issue's summary.

CommentFileSizeAuthor
#6 document_each_use_of-2423951-6.patch4 KBjoelpittet
rest-document-base-scheme-usage.patch3.85 KBeffulgentsia
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/rest/src/LinkManager/TypeLinkManager.php
@@ -41,18 +41,21 @@ public function __construct(CacheBackendInterface $cache, UnroutedUrlAssemblerIn
   public function getTypeUri($entity_type, $bundle) {
-    // @todo Make the base path configurable.

Let's ensure to not drop the @todo, its still valid after this patch, isn't it?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Needs work

.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Status: Needs work » Needs review

Let's ensure to not drop the @todo, its still valid after this patch, isn't it?

I don't think that @todo is still relevant. The new comments say that if you want to override what the method returns, you can subclass it. I'm not aware of any plans to make core's implementation configurable. I left a comment in #1924220-24: Support serialization in hal+json in case anyone there thinks otherwise.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs review » Reviewed & tested by the community

Makes sense to me.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, rest-document-base-scheme-usage.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review
Issue tags: +Documentation
FileSize
document_each_use_of-2423951-6.patch4 KB

The @todo was removed already, Here's a re-roll.

ankur7213’s picture

ankur7213 CreditAttribution: ankur7213 commented

Looks good to me.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs review » Reviewed & tested by the community

Agreed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 6fb9d19 and pushed to 8.0.x. Thanks!

  • alexpott committed 6fb9d19 on 8.0.x 
    Issue #2423951 by effulgentsia, joelpittet, dawehner, klausi: Document...

Status: Fixed » Closed (fixed)

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