Problem/Motivation

A few places, the @return documentation doesn't include the full namespace starting with \Drupal

Proposed resolution

We should fix this, and make the documentation more consistant

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#4 interdiff.txt1.37 KBgoogletorp
#4 use_full_namespace_for-2472453-4.patch1.4 KBgoogletorp
#1 use_full_namespace_for-2472453-1.patch1.4 KBgoogletorp

Comments

googletorp’s picture

googletorp commented
Status: Active » Needs review
StatusFileSize
new use_full_namespace_for-2472453-1.patch1.4 KB
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks for the patch! Needs a bit of work though...

  1. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayModeInterface.php
@@ -28,7 +28,7 @@ public function getTargetType();
-   * @return Drupal\Core\Entity\EntityDisplayModeInterface
+   * @return \Drupal\Core\Entity\EntityDisplayModeInterface
    *   The display mode object, for fluent interface.
    */
   public function setTargetType($target_entity_type);
diff --git a/core/modules/system/src/Tests/DrupalKernel/DrupalKernelTest.php b/core/modules/system/src/Tests/DrupalKernel/DrupalKernelTest.php

    Is this returning a new instance of the class, or the current instance of the class?

    If it's the current one, it should say

    @return $this

    with no description.

    If it's a new instance, it should say

    @return static

  2. +++ b/core/modules/system/src/Tests/DrupalKernel/DrupalKernelTest.php
@@ -57,7 +57,7 @@ protected function prepareConfigDirectories() {
    *   Build the kernel in a read only state.
-   * @return DrupalKernel
+   * @return \Drupal\Core\DrupalKernel
    */

    Can you also add a blank line before the @return? Also this @return needs documentation.

googletorp’s picture

googletorp commented

Thanks for the review.

Regarding setTargetType, for all entity interfaces that I've seeen (like NodeInterface) setters return $this, but the interface documentation is like the above example, with @return \Drupal\node\NodeInterface. Does that mean, that this is also wrong, and the proper documentation in this case would be @return $this instead?

/**
   * Sets the node sticky status.
   *
   * @param bool $sticky
   *   TRUE to set this node to sticky, FALSE to set it to not sticky.
   *
   * @return \Drupal\node\NodeInterface
   *   The called node entity.
   */
  public function setSticky($sticky);
googletorp’s picture

googletorp commented
Status: Needs work » Needs review
StatusFileSize
new use_full_namespace_for-2472453-4.patch1.4 KB
new interdiff.txt1.37 KB

Added new patch based on comments from jhodgdon.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Looks fine, thanks!

  • xjm committed e9d6bd7 on 8.0.x 
    Issue #2472453 by googletorp, jhodgdon: Use full namespace for @return...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -consistency

Thanks @googletorp for the patch. It looks correct to me as well.

This issue only changes documentation, so per https://www.drupal.org/core/beta-changes, this can be completed any time during the Drupal 8 beta phase. Committed and pushed to 8.0.x.

Status: Fixed » Closed (fixed)

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