Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#41927 closed defect (bug) (fixed)

Twenty Seventeen: Missing @return in comment documentation

Reported by: ndoublehwp's profile ndoublehwp Owned by: sergeybiryukov's profile 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 6 years ago.
In the comment docs of function twentyseventeen_video_controls(), the @return is missing
41927.2.diff (861 bytes) - added by audrasjb 6 years ago.
Add inline doc before the function
41927.3.diff (850 bytes) - added by audrasjb 6 years ago.
remove $settings from return inline doc

Download all attachments as: .zip

Change History (13)

@ndoublehwp
6 years ago

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

#1 @audrasjb
6 years ago

  • Keywords has-patch added

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

Regards,
Jb

Version 0, edited 6 years ago by audrasjb (next)

@audrasjb
6 years ago

Add inline doc before the function

#2 @truongwp
6 years ago

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

#3 @audrasjb
6 years 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
6 years ago

remove $settings from return inline doc

#4 @xkon
6 years 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 6 years ago by xkon (previous) (diff)

#5 @audrasjb
6 years 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
6 years 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
6 years 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
6 years ago

  • Milestone changed from Awaiting Review to 4.8.3

#9 @SergeyBiryukov
6 years ago

  • Milestone changed from 4.8.3 to 4.9

#10 @SergeyBiryukov
6 years 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.