Use full namespace for @return documentation [#2472453]

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

Comment	File	Size	Author
#4	interdiff.txt	1.37 KB	googletorp
#4	use_full_namespace_for-2472453-4.patch	1.4 KB	googletorp
#4
#1	use_full_namespace_for-2472453-1.patch	1.4 KB	googletorp
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

googletorp CreditAttribution: googletorp commented 16 April 2015 at 14:43

Status:

Active

» Needs review

File	Size
use_full_namespace_for-2472453-1.patch	1.4 KB

Comment #2

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 17 April 2015 at 15:33

Status:

Needs review

» Needs work

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

+++ 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

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

Comment #3

googletorp CreditAttribution: googletorp commented 19 April 2015 at 07:04

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);

Comment #4

googletorp CreditAttribution: googletorp commented 19 April 2015 at 07:09

Status:

Needs work

» Needs review

File	Size
use_full_namespace_for-2472453-4.patch	1.4 KB

interdiff.txt	1.37 KB

Added new patch based on comments from jhodgdon.

Comment #5

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 23 April 2015 at 17:43

Status:

Needs review

» Reviewed & tested by the community

Looks fine, thanks!

Comment #6

24 April 2015 at 01:27

xjm committed e9d6bd7 on 8.0.x

Issue #2472453 by googletorp, jhodgdon: Use full namespace for @return...

Comment #7

xjm

she/her

English

CreditAttribution: xjm at Acquia commented 24 April 2015 at 01:27

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.