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\Component is the part of Drupal that's the most outwards facing, so it makes sense to start from there.

I will reroll any patch broken by these changes.

CommentFileSizeAuthor
#22 2471571-22-standardize-getter-docblocks.patch22.96 KBbojanz
#11 2471571-11-standardize-getter-docblocks.patch22.66 KBbojanz
#9 2471571-9-standardize-getter-docblocks.patch22.7 KBbojanz
#6 2471571-6-standardize-getter-docblocks.patch22.26 KBbojanz
#4 example-of-missings.txt9.35 KBDom.
#3 interdiff.txt684 bytesbojanz
#3 2471571-2-standardize-getter-docblocks.patch16.8 KBbojanz
#1 2471571-1-standardize-getter-docblocks.patch16.8 KBbojanz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2471571-1-standardize-getter-docblocks.patch16.8 KB
anavarre’s picture

anavarre
Primary language French
Location 🇪🇺
CreditAttribution: anavarre at Acquia commented 
+++ b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php
@@ -162,7 +162,14 @@ public function delete($name) {
-   * Gets the full path of the containing directory where the file is or should be stored.

Should be wrapped at 80 cols.

Else this looks good to me.

bojanz’s picture

bojanz CreditAttribution: bojanz commented
FileSize
2471571-2-standardize-getter-docblocks.patch16.8 KB
interdiff.txt684 bytes

Addressed #2.

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Needs review » Needs work
FileSize
example-of-missings.txt9.35 KB

Hi !
For me, a lot are missing to follow the convention strictly. I just added some of those per example in the file attached. What do you think ?

anavarre’s picture

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

bojanz CreditAttribution: bojanz commented
Status: Needs work » Needs review
FileSize
2471571-6-standardize-getter-docblocks.patch22.26 KB

I've fixed the "Get" instances.

Dom.’s picture

Dom. CreditAttribution: Dom. commented

Still some missings.... sorry
Example DataTimePLus.php :

  /**
   * Retrieves error messages.
   *
   * Public function to return the error messages.
   */
  public function getErrors() {
    return $this->errors;
  }

You could look at all occurences of "function get" in Drupal/Component folder.

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Needs review » Needs work
bojanz’s picture

bojanz CreditAttribution: bojanz commented
Status: Needs work » Needs review
FileSize
2471571-9-standardize-getter-docblocks.patch22.7 KB

Done that one as well.
I'm stopping here, it's fine if we miss a few in the initial go, as long as most of them are covered.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php
@@ -162,7 +162,15 @@ public function delete($name) {
+   * Gets the full path of the containing directory where the file is or should
+   * be stored.

So strictly we have the one line thing for the first row, should we tackle that here?

bojanz’s picture

bojanz CreditAttribution: bojanz commented
FileSize
2471571-11-standardize-getter-docblocks.patch22.66 KB

Changed it to "Gets the full path of the file's containing directory.", the full description remains in @returns.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Component: base system » documentation
Status: Needs review » Reviewed & tested by the community

Thank you!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2471571-11-standardize-getter-docblocks.patch, failed testing.

bojanz’s picture

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

Come on, testbot.

bojanz queued 11: 2471571-11-standardize-getter-docblocks.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 11: 2471571-11-standardize-getter-docblocks.patch, failed testing.

Status: Needs work » Needs review

AjitS queued 11: 2471571-11-standardize-getter-docblocks.patch for re-testing.

bojanz’s picture

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

And we're back.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2471571-11-standardize-getter-docblocks.patch, failed testing.

Status: Needs work » Needs review

bojanz queued 11: 2471571-11-standardize-getter-docblocks.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 11: 2471571-11-standardize-getter-docblocks.patch, failed testing.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs work » Reviewed & tested by the community
FileSize
2471571-22-standardize-getter-docblocks.patch22.96 KB

Rebased.

  • xjm committed 20e4eda on 8.0.x 
    Issue #2471571 by bojanz, Dom.: Standardize getter docblocks in Drupal\...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed

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. Thanks!

Status: Fixed » Closed (fixed)

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