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 Language 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 #1533248: Make language module pass Coder Review
#1310084: [meta] API documentation cleanup sprint #1392962: Clean up API docs for the language module
#500866: [META] remove t() from assert message #1797360: Remove t() from assertion messages in tests for the language module
CommentFileSizeAuthor
#9 1811214-9.patch6.2 KBnaveenvalecha
#7 1811214-language-blockdocs-7.patch10.76 KBtisteegz
#3 1811214_3.patch10.79 KBMile23
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented
Title: Add missing type hinting to Node module docblocks » Add missing type hinting to Language module docblocks

Correcting the title...

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 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
CreditAttribution: Mile23 commented
Status: Active » Needs review
FileSize
1811214_3.patch10.79 KB

Something to chew on.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue tags: +Needs reroll
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs review » Needs work
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes
tisteegz’s picture

tisteegz CreditAttribution: tisteegz commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1811214-language-blockdocs-7.patch10.76 KB

Re-rolled patch.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Version: 8.0.x-dev » 8.1.x-dev
Status: Needs review » Needs work
Issue tags: +Needs reroll

Bumping to 8.1.x. Patch no longer applies.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1811214-9.patch6.2 KB
madhavvyas’s picture

madhavvyas CreditAttribution: madhavvyas as a volunteer and at CIGNEX commented

Code looks good to me.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Cool, thanks.

Patch applies, phpcs says no problems.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Version: 8.1.x-dev » 8.0.x-dev

+1 on RTBC. Moving back to 8.0.x though as there is no reason this cannot be committed there too.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

  • catch committed ae82707 on 8.1.x 
    Issue #1811214 by Mile23, naveenvalecha, tisteegz: Add missing type...

  • catch committed 7181085 on 8.0.x 
    Issue #1811214 by Mile23, naveenvalecha, tisteegz: Add missing type...

Status: Fixed » Closed (fixed)

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