WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 7 weeks ago

#41927 closed defect (bug) (fixed)

Twenty Seventeen: Missing @return in comment documentation

Reported by: ndoublehwp Owned by: SergeyBiryukov
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.9
Component: Bundled Theme Keywords: has-patch
Focuses: docs Cc:

Description

In the comment docs of function

twentyseventeen_video_controls()

, the @return is missing

Attachments (3)

41927.diff (769 bytes) - added by ndoublehwp 2 months ago.
In the comment docs of function twentyseventeen_video_controls(), the @return is missing
41927.2.diff (861 bytes) - added by audrasjb 2 months ago.
Add inline doc before the function
41927.3.diff (850 bytes) - added by audrasjb 2 months ago.
remove $settings from return inline doc

Download all attachments as: .zip

Change History (13)

@ndoublehwp
2 months ago

In the comment docs of function twentyseventeen_video_controls(), the @return is missing

#1 @audrasjb
2 months ago

  • Keywords has-patch added

Hi ndoublehwp,
Nice catch ;)
Here is a patch to include inline doc before this function.

Regards,
Jb

EDIT: ow… you auto answered… ok, sorry, nevermind.

Last edited 2 months ago by audrasjb (previous) (diff)

@audrasjb
2 months ago

Add inline doc before the function

#2 @truongwp
2 months ago

@audrasjb I think we don't need $settings in @return line

#3 @audrasjb
2 months ago

@truongwp ha? Ok. Is it better now? This is my very first step in inline doc so please let me know if anything is wrong :)

@audrasjb
2 months ago

remove $settings from return inline doc

#4 @xkon
2 months ago

@truongwp, usually we state what returns though. So shouldn't it actually be @return Settings or something similar on this instance?


Edit: @truongwp is correct. I guess I've missed it before... x_x

@audrasjb check this : https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#4-hooks-actions-and-filters

Last edited 2 months ago by xkon (previous) (diff)

#5 @audrasjb
2 months ago

So @xkon @truongwp if I not misunderstand the handbook, there should not be any @return in inline documentation because filters always return their first parameter. Am I right?

#6 @xkon
2 months ago

According to the Handbook

Quoting:

Note that @return is not used for hook documentation, because action hooks return nothing, and filter hooks always return their first parameter.

So yes the original documentation was fine as it was since this is a filter function.

#7 @truongwp
2 months ago

Hi,

This is inline docs for a function. Please see the example here: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-functions-class-methods

@return type Description. or @return type are correct, not @return type $var Description. or @return Description.

#8 @SergeyBiryukov
7 weeks ago

  • Milestone changed from Awaiting Review to 4.8.3

#9 @SergeyBiryukov
7 weeks ago

  • Milestone changed from 4.8.3 to 4.9

#10 @SergeyBiryukov
7 weeks ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 41780:

Twenty Seventeen: Add missing @return tag for twentyseventeen_video_controls().

Props ndoublehwp, audrasjb.
Fixes #41927.

Note: See TracTickets for help on using tickets.