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 Locale 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 (20-25 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 #1533250: Many coding standards clean-ups in locale module
#1310084: [meta] API documentation cleanup sprint #1326618: Clean up API docs for locale module
#500866: [META] remove t() from assert message #1797364: Remove t() from assertion messages in tests for the locale module
CommentFileSizeAuthor
#9 1807062_9.patch2.25 KBmile23
#5 1807062_5.patch3.66 KBmile23
#2 1807062_2.patch18.13 KBmile23

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 1807062_2.patch18.13 KB

Behold.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Related issues: +#2411767: locale_translation_check_projects() should return batch or NULL.

Note that for locale_translation_check_projects() I filed this issue: #2411767: locale_translation_check_projects() should return batch or NULL.

Status: Needs review » Needs work

The last submitted patch, 2: 1807062_2.patch, failed testing.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Component: locale.module » documentation
Status: Needs work » Needs review
StatusFileSize
new 1807062_5.patch3.66 KB

Ignore previous patch. This one only changes docblocks, and so therefore this is a documentation issue.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Still applies.

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
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Version: 8.0.x-dev » 8.1.x-dev
StatusFileSize
new 1807062_9.patch2.25 KB

Reroll of #5.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

sk33lz’s picture

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

Patch #9 looks good and still applies against 8.6.x.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Let's re-run the test against 8.6.x to see if it fails again...

alexpott’s picture

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

Committed to 8.5.x as this is docs only.

Committed and pushed 052a2b369f to 8.6.x and eab1b2fe6b to 8.5.x. Thanks!

  • alexpott committed 36d5f79 on 8.6.x 
    Issue #1807062 by Mile23: Add missing type hinting to Locale module...

  • alexpott committed eab1b2f on 8.5.x 
    Issue #1807062 by Mile23: Add missing type hinting to Locale module...

Status: Fixed » Closed (fixed)

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