This is a sub-issue of #1800046: [META] Add missing type hinting to core docblocks, fix Drupal.Commenting.FunctionComment.Missing* focused on correctly adding @param and @return type hinting to the Statistics module.

Documentation patches that include type hinting are time consuming to both review and commit because one must dig into the actual code to confirm that the type hints are both correct and complete. Hence, please be patient and try to limit type hint patches to covering only a limited number of docblocks (10-15 as a guess).

How To Review This Issue

  1. Attempt to apply the patch to see if it needs a reroll.
  2. Use the phpcs one-liner to evaluate whether all the relevant standards errors have been resolved: https://gist.github.com/paul-m/227822ac7723b0e90647
  3. Look at each change and determine whether the type hint is correct.

Related sprint issues:

Sprint Topic Sub Issue
#1518116: [meta] Make Core pass Coder Review #1533390: Make statistics module pass Coder Review
#1310084: [meta] API documentation cleanup sprint #1313510: Clean up API docs for statistics module
#500866: [META] remove t() from assert message #1797520: Remove t() from assertion messages in tests for the statistics module
CommentFileSizeAuthor
#2 1811892_2.patch942 bytesmile23

Comments

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
Parent issue: » #1800046: [META] Add missing type hinting to core docblocks, fix Drupal.Commenting.FunctionComment.Missing*
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 1811892_2.patch942 bytes

Not a lot to worry about here.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Still applies.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +india, +SprintWeekend2015

Looking good to me.
Converting to RTBC

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed 
+++ b/core/modules/statistics/statistics.module
@@ -148,7 +148,7 @@ function statistics_get($nid) {
-  // clean up statistics table when node is deleted
+  // Clean up statistics table when node is deleted.

Changing this comment is scope creep, so technically not correct in this patch, but I chose to commit it in this case. However, please be sure in the future to restrict cleanups and fixes to what specified in the issue scope. Thanks! Committed and pushed to 8.0.x.

  • xjm committed 161ddb9 on 8.0.x 
    Issue #1811892 by Mile23: Add missing type hinting to Statistics module...

Status: Fixed » Closed (fixed)

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