WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#25451 closed defect (bug) (fixed)

status_header() incorrect return statement

Reported by: tivnet Owned by: SergeyBiryukov
Milestone: 3.8 Priority: normal
Severity: trivial Version: 2.0
Component: Inline Docs Keywords: has-patch commit
Focuses: Cc:

Description

Attempt to return @header(... while header() is void.

Attachments (3)

status_header_incorrect_return.patch (803 bytes) - added by tivnet 7 years ago.
status_header_incorrect_return_1.patch (896 bytes) - added by tivnet 7 years ago.
Version w/o @return
25451.patch (1.9 KB) - added by SergeyBiryukov 7 years ago.

Download all attachments as: .zip

Change History (11)

#1 follow-ups: @SergeyBiryukov
7 years ago

  • Component changed from General to Inline Docs

Should we perhaps just omit the return (like in #25450), to keep the current behaviour?

#2 in reply to: ↑ 1 @tivnet
7 years ago

Replying to SergeyBiryukov:

Should we perhaps just omit the return (like in #25450), to keep the current behaviour?

In this specific function, boolean return can make some sense, in my opinion.
For example, we can add headers_sent() validation, and return false if already sent?

You are right, the simplest patch would be indeed just omitting the return. But, do we expect anyone to really know if the method worked? If yes, then boolean return can be a good start...

@tivnet
7 years ago

Version w/o @return

#3 in reply to: ↑ 1 @tivnet
7 years ago

Replying to SergeyBiryukov:

Should we perhaps just omit the return (like in #25450), to keep the current behaviour?

Made a new patch. Please take a look. Thank you!

#4 in reply to: ↑ 1 @tivnet
7 years ago

Replying to SergeyBiryukov:

Sergey, could you please make a resolution on this? The most recent patch does not have the @return.
Thank you!

#5 @SergeyBiryukov
7 years ago

  • Milestone changed from Awaiting Review to 3.8

#6 follow-up: @SergeyBiryukov
7 years ago

  • Keywords commit added

25451.patch is another take. Renamed some variables for clarity. Removed @uses apply_filters(), as the filter itself should be documented instead.

#7 in reply to: ↑ 6 @DrewAPicture
7 years ago

Replying to SergeyBiryukov:

25451.patch is another take. Renamed some variables for clarity. Removed @uses apply_filters(), as the filter itself should be documented instead.

25451.patch works for me. +1 for documenting with code :)

#8 @SergeyBiryukov
7 years ago

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

In 25990:

Remove incorrect @return value from status_header(). Rename some variables for clarity.

props tivnet for initial patch.
fixes #25451.

Note: See TracTickets for help on using tickets.