WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

#26185 closed defect (bug) (fixed)

PHPDoc updates for /wp-includes/functions.php

Reported by: morganestes Owned by: DrewAPicture
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.7
Component: Bootstrap/Load Keywords: has-patch
Focuses: docs Cc:

Description

Style fixes to add periods, arrange keywords in the correct order, and correct @param and @return values (adding descriptions when needed).

Attachments (8)

26185.diff (33.1 KB) - added by morganestes 8 years ago.
26185.1.diff (33.2 KB) - added by morganestes 8 years ago.
Refresh
26185.date_i18n.diff (827 bytes) - added by morganestes 8 years ago.
date_i18n
26185._http_build_query.diff (1.1 KB) - added by morganestes 8 years ago.
Added complete documentation of _http_build_query()
26185.wp_auth_check.diff (611 bytes) - added by morganestes 8 years ago.
Added parameter and return tags to wp_auth_check().
26185.wp_checkdate.diff (758 bytes) - added by morganestes 8 years ago.
Added parameter and return tags to wp_checkdate.
26185.rollup.diff (2.7 KB) - added by morganestes 8 years ago.
Consolidated patch with 4 functions documented.
26185.2.diff (1.3 KB) - added by TobiasBg 7 years ago.

Download all attachments as: .zip

Change History (27)

@morganestes
8 years ago

#1 @morganestes
8 years ago

  • Version 3.8 deleted

#2 @morganestes
8 years ago

  • Version set to trunk

#3 @nacin
8 years ago

  • Component changed from Inline Docs to Bootstrap/Load
  • Focuses docs added

#4 follow-up: @jeremyfelt
8 years ago

  • Keywords needs-refresh added

Looks good, @morganestes.

A couple notes from a read through:

  • docs for _http_build_query() seem to have some spacing issues around the params
  • similar spacing issues in docs for wp_filter_object_list()

@morganestes
8 years ago

Refresh

#5 in reply to: ↑ 4 @morganestes
8 years ago

  • Keywords 2nd-opinion added; needs-refresh removed
  • Version changed from trunk to 3.7

Replying to jeremyfelt:

  • docs for _http_build_query() seem to have some spacing issues around the params
  • similar spacing issues in docs for wp_filter_object_list()

Thanks for checking it out. I adjusted the spacing on those two doc blocks and added it as a .1 diff.

#6 follow-up: @DrewAPicture
8 years ago

  • Keywords needs-patch added; has-patch 2nd-opinion removed

Ideally, I'd like to avoid these huge patches in the future, instead focusing on specific change. It's tough to follow what's being changed and why, etc.

That said, here are some notes on 26185.1.diff:

Reference: Inline Documentation Standards

  • There should always be a new blank line between the short and long descriptions in a doc block
  • If you're going to add parameter descriptions, great. I'd prefer it if you consistently space-aligned the parts for parameters. Case in point, compare your docs changes for date_i18n() vs deprecated_file(). The style used with date_i18n() is definitely preferred :)
  • Please avoid adding new @global tags
  • @link, @see, etc. tags should always fall below the @since
  • If parameter description is wrapped, align with the first character of the first line with spaces
  • For hash notations (arrays), use a 4-space indent for each level of the array, not tabs

#7 in reply to: ↑ 6 @morganestes
8 years ago

Replying to DrewAPicture:

Ideally, I'd like to avoid these huge patches in the future, instead focusing on specific change. It's tough to follow what's being changed and why, etc.

Is it better to have a separate patch for each function/hook that I'm documenting?

I'll look at the overall structure again. I relied too much on my IDE autogenerating and formatting and it bit me. I'll go back and try again.

#8 follow-up: @morganestes
8 years ago

I'm going back through this after 3.9 and will try again. This time, I'm adding a diff per function with major changes, and a combined one for just whitespace changes.

@morganestes
8 years ago

date_i18n

@morganestes
8 years ago

Added complete documentation of _http_build_query()

#9 in reply to: ↑ 8 ; follow-up: @DrewAPicture
8 years ago

  • Milestone changed from Awaiting Review to Future Release

Replying to morganestes:

I'm going back through this after 3.9 and will try again. This time, I'm adding a diff per function with major changes, and a combined one for just whitespace changes.

Great. Honestly, I'd prefer you group up PHPDoc changes for multiple functions in patches. You don't need to do the whole file in one patch, but I'd rather not have 40 tiny patches either.

Also, you can skip the whitespace changes. The only time whitespace changes are really accepted is when they're on lines already affect by code-level changes. This is to prevent unnecessary churn in the codebase.

@morganestes
8 years ago

Added parameter and return tags to wp_auth_check().

@morganestes
8 years ago

Added parameter and return tags to wp_checkdate.

#10 in reply to: ↑ 9 @morganestes
8 years ago

Replying to DrewAPicture:

Great. Honestly, I'd prefer you group up PHPDoc changes for multiple functions in patches. You don't need to do the whole file in one patch, but I'd rather not have 40 tiny patches either.

Sorry, didn't see this until I started adding the individual patches. I'll merge then into one more patch, and that should be it for this ticket.

@morganestes
8 years ago

Consolidated patch with 4 functions documented.

#11 @morganestes
8 years ago

  • Keywords has-patch added; needs-patch removed

#12 @wonderboymusic
7 years ago

  • Owner set to DrewAPicture
  • Status changed from new to reviewing

This might be 4.0 material

#13 @DrewAPicture
7 years ago

In 28917:

Improve inline documentation for date_i18n(), _http_build_query(), wp_checkdate(), and wp_auth_check().

Props morganestes.
See #26185.

#14 @DrewAPicture
7 years ago

In 28918:

General inline documentation improvements in wp-includes/functions.php.

First run. See #26185.

#15 @DrewAPicture
7 years ago

In 28936:

General inline documentation improvements in wp-includes/functions.php.

Second run. See #26185.

#16 @SergeyBiryukov
7 years ago

In 28937:

Some fixes for get_weekstartend() docs.

see #26185.

#17 @DrewAPicture
7 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 29011:

General inline documentation improvements in wp-includes/functions.php.

Final run. Fixes #26185.

#18 @TobiasBg
7 years ago

  • Milestone changed from Future Release to 4.0
  • Resolution fixed deleted
  • Status changed from closed to reopened

Found two inconsistencies in [29011], in the grammar for the _deprecated_*() functions, plus coding standards in the example PHP code, see 26185.2.diff.

@TobiasBg
7 years ago

#19 @DrewAPicture
7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 29013:

Fix inconsistent language in two function descriptions in wp-includes/functions.php.

Also adds correct coding standards to a code sample in _deprecated_argument().

Props TobiasBg.
Fixes #26185.

Note: See TracTickets for help on using tickets.