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 Config 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 #1533202: Make config module pass Coder Review
#1310084: [meta] API documentation cleanup sprint #1541930: Clean up API docs for config module
#500866: [META] remove t() from assert message #1797220: Remove t() from asserts from Entity sub-system tests
CommentFileSizeAuthor
#13 drupal8_—_bash_—_204×63.jpg77.59 KBdeepakaryan1988
#8 1810002-7.patch1.6 KBpiyuesh23
#1 1810002_1.patch1.6 KBmile23

Comments

lars toomre’s picture

lars toomre commented
Issue summary: View changes

Updated issue summary.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 1810002_1.patch1.6 KB

Short but sweet.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
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
Issue summary: View changes
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
Status: Needs review » Reviewed & tested by the community

Looking nice to me!
Changing the status to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: 1810002_1.patch, failed testing.

Status: Needs work » Needs review

JacobSanford queued 1: 1810002_1.patch for re-testing.

piyuesh23’s picture

piyuesh23 commented
Issue tags: +Needs reroll

Patch needs a re-roll.

piyuesh23’s picture

piyuesh23 commented
StatusFileSize
new 1810002-7.patch1.6 KB

Uploading an updated patch here.

mac_weber’s picture

mac_weber commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 1810002-7.patch, failed testing.

Status: Needs work » Needs review

Mac_Weber queued 8: 1810002-7.patch for re-testing.

mac_weber’s picture

mac_weber commented
Status: Needs review » Reviewed & tested by the community
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
StatusFileSize
new drupal8_—_bash_—_204×63.jpg77.59 KB

@piyuesh23 I don't know whether it's right or wrong but while applying your patch I am getting a warning.
Attaching screenshot. Please have a look.

alexpott’s picture

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

Docs only change - permitted under the beta evaluation. Committed 0504dd9 and pushed to 8.0.x. Thanks!

  • alexpott committed 0504dd9 on 8.0.x 
    Issue #1810002 by Mile23, piyuesh23: Add missing type hinting to Config...

Status: Fixed » Closed (fixed)

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

piyuesh23’s picture

piyuesh23 commented