As described in #2471485: Standardize getter docblocks we want to standardize on using "Gets" instead of "Returns" on getter docblocks, matching the outside world and making core internally consistent. In general, we make the verb match the one in the method name.

Drupal\Core\Entity is one of the biggest offenders in core (compared to Config which uses "Gets" almost exclusively, for example), and also the one most visible to contrib, since the docblocks here cascade into contrib entity types.

I will reroll any patch broken by these changes (just list them in this issue or the parent meta).

CommentFileSizeAuthor
#7 2471653-6-standardize-getter-docblocks.patch55.18 KBKazanir
#1 2471653-1-standardize-getter-docblocks.patch55.47 KBbojanz

Comments

bojanz’s picture

bojanz commented
Status: Active » Needs review
StatusFileSize
new 2471653-1-standardize-getter-docblocks.patch55.47 KB
anavarre’s picture

anavarre
Primary language French
Location 🇪🇺
commented 
+++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
@@ -383,7 +383,7 @@ public function resetCache(array $entities = NULL) {
+   * Determines whether the view mode is cacheable.

Isn't this a DX regression to not indicate that this should return TRUE somehow?

Else this looks good to me.

anavarre’s picture

anavarre
Primary language French
Location 🇪🇺
commented
Issue tags: +DX (Developer Experience), +consistency
bojanz’s picture

bojanz commented

@#2
No, we use the same wording in many places across core. The boolean return is both implicit in the wording, and documented separately via @return

Kazanir’s picture

Kazanir commented
Status: Needs review » Reviewed & tested by the community

Looks like "returns" is being used as a substitute for what the function actually does (the replacements range from "gets" to "determines" to "builds" to "instantiates") in several different types of functions. Getting rid of that ambiguity (and standardizing getters) is a good move.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: 2471653-1-standardize-getter-docblocks.patch, failed testing.

Kazanir’s picture

Kazanir commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new 2471653-6-standardize-getter-docblocks.patch55.18 KB

Needs a slight reroll to apply actually...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Priority: Normal » Minor
Status: Reviewed & tested by the community » Fixed

Committed 7171165 and pushed to 8.0.x. Thanks!

  • alexpott committed 7171165 on 8.0.x 
    Issue #2471653 by bojanz, Kazanir: Standardize getter docblocks in...

Status: Fixed » Closed (fixed)

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