Add Inline Docs for Hooks

[rzen]

Document ALL THE HOOKS!

This trac ticket corresponds with a ​make/core blog post of the same name, an initative we're kicking off in 3.7, as discussed in ​this earlier make/core post.

Get Involved!

We'd love your help contributing! You can get started by following the "how to contribute" section and leaving a comment on ​the make/core post.

Keeping Discussions Focused

Any discussion about the specifics of a patch itself (e.g. "it should say this") should happen here on trac, like it always does. Any discussion about the broader scope of what we’re trying to do (e.g. "all hook docs should do this") should take place on the ​make/core P2.

Keeping Your Patch Clean

You may have claimed several files, but it would probably be best if you submitted individual patches for each file separately. That makes the changeset easier to grok, and the patch easeir to apply.

[rzen]

Second pass at ./wp-comments-post.dff

[DrewAPicture]

@rzen: wp-comments-post.diff​. Looks like your @param lines need periods :)

[natejacobs]

First pass at wp-admin/admin-footer.php

[natejacobs]

First pass at wp-admin/index.php

[bananastalktome]

Pass at wp-trackback.php (not 100% on the @since, but pretty sure)

[nacin]

In 25249:

Inline documentation for hooks in wp-comments-post.php.

props rzen.
see #25229.

[nacin]

In 25250:

Inline documentation for the welcome_panel hook.

props natejacobs.
see #25229.

[nacin]

In 25251:

Fixes for hook inline docs in wp-comments-post.php. see #25229.

[nacin]

In 25252:

Hook docs for admin-footer.php.

props natejacobs.
see #25229.

[nacin]

In 25253:

Document the trackback_post hook in wp-trackback.php.

props bananastalktome.
see #25229.

[nacin]

In 25273:

Short descriptions for inline docs should end with a period, per the vast majority of core. see #25229.

[bftrick]

first pass at xmlrpc.php

[dustyf]

first pass at wp-admin/comment.php

[dustyf]

First pass at wp-admin/custom-background.php

[natejacobs]

wp-includes/wp-db.php

[natejacobs]

wp-includes/theme-compat/comments.php

[natejacobs]

wp-includes/theme-compat/comments.php

[natejacobs]

I wasn't sure of the best way to handle the use of the

do_action('comment_form');

in the deprecated wp-includes/theme-compat/ files. I added a note in the long description that this use of the action is in a deprecate file. Better to use the @deprecated tag in this case?

[natejacobs]

wp-includes/rss.php

[nacin]

Replying to natejacobs:

I wasn't sure of the best way to handle the use of the

do_action('comment_form');

in the deprecated wp-includes/theme-compat/ files. I added a note in the long description that this use of the action is in a deprecate file. Better to use the @deprecated tag in this case?

This is actually a //duplicate_hook situation. comment_form is a hook included in the comment_form() function.

[nacin]

I would avoid wp-includes/theme-compat/ all together.

[nacin]

In 25281:

Inline documentation for hooks in xmlrpc.php.

The old link for RSD is dead; update to the new one.

props bftrick.
see #25229.

[nacin]

In 25282:

Document comment_edit_redirect.

props dustyf.
see #25229.

[nacin]

In 25283:

Mark the hooks in custom-background.php as duplicates.

• image_size_names_choose should be documented in wp-includes/media.php
• wp_create_file_in_uploads should be documented in custom-header.php

see #25229.

[nacin]

In 25284:

Document the 'query' filter in wp-db.

props natejacobs.
see #25229.

[nacin]

In 25286:

Inline docs for hooks in MagPie.

props natejacobs.
see #25229.

[nacin]

In 25288:

Correct @since in admin-footer. see #25229.

[Faison]

First pass at wp-includes/nav-menu-template.php, questions to follow.

[Faison]

I have a couple of questions regarding the diff I submitted.

First, regarding the nav_menu_link_attributes filter: the first argument is an array that I feel could benefit from having it's structure documented similar to the way Argument arrays are supposed to be documented. So I documented it like it was an Arguments array, but I wanted to know if that is how it should be done?

Second, I wanted to know if I'm documenting the $args parameter properly, with a @see pointing towards the originating function.

And last, do we have a standard for how to document a parameter that is a field in an object? For example, some actions have a parameter like $foo->bar, so do we document it as @param type $foo->bar description. ?

Thanks for all the help,
Faison

[Faison]

Second Pass, forgot to add description for walker_nav_menu_start_el's first parameter

[nacin]

In 25290:

Document the event hook in wp-cron.php. see #25229.

[Frank Klein]

Adds filter documentation for wp-includes/author-template.php. Also updates outdated function documentation.

[dustyf]

Question: For a duplicate hook, how do we know which instance of the hook should be documented and which should be marked duplicate? Also, I know the standard being used is // Duplicate hook , but would it make sense to have some reference to where it is documented so someone could more easily find it?

[DrewAPicture]

Replying to dustyf:

Question: For a duplicate hook, how do we know which instance of the hook should be documented and which should be marked duplicate? Also, I know the standard being used is // Duplicate hook , but would it make sense to have some reference to where it is documented so someone could more easily find it?

The standard being used is very specifically //duplicate_hook (no space, underscore between the words).

And you'd want to search for multiplies of the same hook tag and svn blame those lines to find the "first" use of it in terms of earliest revision. The first use gets a docblock, the second (third, fourth, etc) get //duplicate_hook.

[enej]

First pass at ms.php

[DrewAPicture]

[25299] adds docs for the wp_link_query and wp_link_query_args filters introduced in [25293].

[tw2113]

wp-includes/http.php done.

[tw2113]

Fixes some inconsistencies noticed by DrewAPicture

[tw2113]

I'll get this right eventually :)

[DrewAPicture]

http.php.2.diff​ looks pretty good. Looks like you still need to do the allowed origins for the allowed_http_origins hook. Take a look at the ​hooks example for how to do this.

[tw2113]

last one

[tw2113]

Capitalization consistency for URL and HTTP. Includes a couple non-hook comments.

[SergeyBiryukov]

In 25302:

Inline documentation for hooks in http.php.

props tw2113.
see #25229.

[nullvariable]

wp-activate.php inline hook documentation, and minor corrections on version for other documentation.

[NikV]

First pass at wp-admin/ms-delete-site.php

[nullvariable]

inline documentation for admin-ajax.php.

[siobhyb]

Second pass at options.php. Thanks @DrewAPicture, @tierra, and @kimparsell.

[DrewAPicture]

options.patch​ looks good to me. Nice first patch @siobhyb!

[siobhyb]

First pass at wp-admin/options-writing.php. 3 of the 4 hooks here are duplicates.

[natejacobs]

Second pass of wp-includes/shortcodes.php. Thanks @drewapicture

[bftrick]

Question. If a hook is in the middle of a block of HTML do you comment above the html or inside of it? I assume above it as it would be cleaner.

Ex. wp-admin/user-new.php

<form action="" method="post" name="adduser" id="adduser" class="validate"<?php do_action('user_new_form_tag');?>>

Another question. What if the hook is embedded in a long conditional?

if ( is_multisite() && current_user_can( 'promote_users' ) && ! wp_is_large_network( 'users' )
	&& ( is_super_admin() || apply_filters( 'autocomplete_users_for_site_admins', false ) )
) {
	wp_enqueue_script( 'user-suggest' );
}

[rmccue]

Replying to bftrick:

Question. If a hook is in the middle of a block of HTML do you comment above the html or inside of it? I assume above it as it would be cleaner.
[...]
Another question. What if the hook is embedded in a long conditional?

From a technical standpoint, ​WP-Parser#28 is tracking the implementation. For conditionals, you should be able to put it before the entire conditional, and likewise for embedded HTML, as it just holds on to the tag until the next action/filter it sees.

[SergeyBiryukov]

In 25372:

Inline documentation for hooks in wp-admin/options.php.

props siobhyb.
see #25229.

[bftrick]

Replying to rmccue:

Replying to bftrick:

Question. If a hook is in the middle of a block of HTML do you comment above the html or inside of it? I assume above it as it would be cleaner.
[...]
Another question. What if the hook is embedded in a long conditional?

From a technical standpoint, ​WP-Parser#28 is tracking the implementation. For conditionals, you should be able to put it before the entire conditional, and likewise for embedded HTML, as it just holds on to the tag until the next action/filter it sees.

thanks for the clarification! :)

[DrewAPicture]

Weekly Triage/Recap – Sept. 11

After yesterdays weekly inline-docs chat, we decided to start doing a weekly triage of patches on this ticket to both:

• Ensure submitters get feedback on remaining patches
• Committers know what's ready to go in

Patches committed

Patch	File	Changeset
wp-comments-post.diff​	wp-comments-post.php	[25249], [25251]
admin-footer.diff​	wp-admin/admin-footer.php	[25252]
index.diff​	wp-admin/index.php	[25250]
wp-trackback.diff​	wp-trackback	[25253]
xmlrpc.diff​	xmlrpc.php	[25281]
wp-comment.diff​	wp-admin/comment.php	[25282]
custom-background.diff​	wp-admin/custom-background.php	[25283]
wp-db.diff​	wp-includes/wp-db.php	[25284]
rss.diff​	wp-includes/rss.php	[25286]
http.php.4.diff​	wp-includes/http.php	[25302]
options.patch​​	wp-admin/options.php	[25372]

Contributions by: rzen, natejacobs, bananastalktome, nacin, bftrick, dustyf, tw2113, siobhyb

Patches in review
ms.diff​ for wp-admin/includes/ms.php — @enej

• @since tags should use the three-digit x.x.x style.
• For the new_admin_email_content and new_user_email_content filters, I'd suggest pullout the email text string into a variable before the filter doc blocks, then referencing that directly in the @param lines
• For short descriptions, focus more on what's being filtered, not why.

  // For instance, this:
  "Allows you to modify the tables that you want to be dropped when the blog is deleted."
  
  //should be more like:
  "Filter the tables to drop when a blog is deleted."
  
  //This:
  "Allows you to modify the upload base directory that is going to be deleted when the blog is deleted."
  
  // should be more like:
  "Filter the upload base directory to be deleted when a blog is deleted."
  

wp-activate.diff​ for wp-activate.php — @nullvariable

• Remove the action long descriptions as they are unnecessary and redundant.
• @since versions should be in the x.x.x 3-digit style
• Remove the functional doc changes, just focus on the actions here :)
• Side note: @MU shouldn't ever be changed since it signifies that the function came from WPMU

admin-ajax.diff​ for wp-admin/admin-ajax.php — @nullvariable

• Just //duplicate_hook will suffice for the admin_init hook
• Short descriptions should start a capital letter and end with a period.
• Add a blank link between the long description and the @link
• @since should use the 3-digit x.x.x style

Patches ready for commit

Patch	File	Props	Changeset	Notes
options-writing.patch​​	wp-admin/options-writing.php	siobhyb	[25411]	Committed by helen
nav-menu-template.2.diff​	wp-includes/nav-menu-template.php	Faison	[25410]	Please add a period to the short description for the wp_nav_menu filter Committed by helen
shortcodes.diff​​	wp-includes/shortcodes.php	natejacobs	[25423]	Committed by SergeyBiryukov
ms-delete-site.php1.diff​	wp-admin/ms-delete-site.php	NikV	[25424]	Committed by SergeyBiryukov
author-template.4.patch​	wp-includes/author-template.php	Frank Klein	[25429]	Committed by SergeyBiryukov
comment-php.2.patch​	wp-admin/includes/comment.php	mordauk	[25434]	Committed by SergeyBiryukov
export-php.2.patch​	wp-admin/export.php	mordauk	[25435]	Committed by SergeyBiryukov
wp-includes-load.php.2.diff​	wp-includes/load.php	mordauk	[25455]	Committed by SergeyBiryukov
user-new.4.diff​	wp-admin/user-new.php	bftrick	[25470]	Committed by SergeyBiryukov
wp-includes-feed-rdf.3.diff​	wp-includes/feed-rdf.php	dustyf	[25479]	Committed by SergeyBiryukov
wp-admin-includes-plugin.2.diff​	wp-admin/includes/plugin.php	dllh	[25474]	Committed by westi
wp-includes-class-wp-admin-bar.php.patch​	wp-includes/class-wp-admin-bar.php	betzster	[25475]	Committed by westi
wp-links-opml.3.diff​	wp-links-opml.php	netweb	[25480]	Committed by SergeyBiryukov
wp-admin-includes-ms.2.diff​	wp-admin/includes/ms.php	enej, DrewAPicture	Ready for commit

[helen]

In 25410:

Inline docs for hooks in wp-includes/nav-menu-template.php. props Faison. see #25229.

[helen]

Faison - for the future, please make patches from either the root of develop or src, and strip trailing whitespace. :)

[helen]

In 25411:

Inline docs for hooks in wp-admin/options-writing.php. props siobhyb. see #25229.

[helen]

In 25412:

No space between and duplicate_hook. see #25229

[NikV]

Second Pass at wp-admin/ms-delete-site.php. Thanks @DrewAPicture.

[bftrick]

adding 5 doc blocks to user-new.php

[Frank Klein]

Updated patch containing only inline documentation changes.

[DrewAPicture]

Edit: @SergeyBiryukov took care of it on commit.

[DrewAPicture]

@Frank Klein: author-template.2.patch​ is looking better. Some notes:

• For the 'the_author_' . $field filter, $field is part of the filter hook, not a parameter. So maybe just explain that that portion of the hook name is dynamic in a long description
• @param string HTML link needs a period at the end
• $author_id The ID of a user. should probably sub 'user' for 'author'
• The $is_multi_author parameter description should probably be something more like "Whether $is_multi_author should evaluate as true"

Thanks for splitting the original patch apart :)

[DrewAPicture]

@bftrick: user-new.diff​ is a good start.

• All short descriptions for hook docs need a period at the end
• Typo in "sanitzed", should probably also mention the value is unslashed here
• "Adds additional tags to the adduser form" should probably be something more like "Fires inside the adduser form tag"

Thanks for the patches.

[SergeyBiryukov]

In 25423:

Inline documentation for hooks in wp-includes/shortcodes.php.

props natejacobs.
see #25229.

[SergeyBiryukov]

In 25424:

Inline documentation for hooks in wp-admin/ms-delete-site.php.

props NikV.
see #25229.

[Frank Klein]

Updated patch according to comment:44

[mordauk]

Inline docs for wp-admin/includes/comment.php. Just one for the comment_edit_pre` filter.

​http://core.trac.wordpress.org/attachment/ticket/25229/comment-php.patch

[DrewAPicture]

@mordauk:

comment-php.patch​ — Per the ​inline docs standards:

• Need a period on the short description
• A blank line after the @since line
• A period at the end of the description on the @param line.

export-php.patch​:

• Describe when the action fires and where. "Bottom of export filters form", should be more like "Fires after the export filters form"
• Periods after short descriptions
• Blank line after @since lines
• @since value should follow the 3-digit, x.x.x style

General comment: Clarity is preferred over succinctness.

[DrewAPicture]

author-template.4.patch​ is ready for commit. Props @Frank Klein.

[mordauk]

wp-admin/includes/comment.php

[mordauk]

wp-admin/export.php

[mordauk]

Replying to DrewAPicture:

@mordauk:

comment-php.patch​ — Per the ​inline docs standards:

• Need a period on the short description
• A blank line after the @since line
• A period at the end of the description on the @param line.

export-php.patch​:

• Describe when the action fires and where. "Bottom of export filters form", should be more like "Fires after the export filters form"
• Periods after short descriptions
• Blank line after @since lines
• @since value should follow the 3-digit, x.x.x style

General comment: Clarity is preferred over succinctness.

Patches refreshed, thanks!

[SergeyBiryukov]

In 25429:

Inline documentation for hooks in wp-includes/author-template.php.

props Frank Klein.
see #25229.

[DrewAPicture]

@mordauk:

In the future, please don't overwrite old patches with the new ones, makes it hard to track progress and context of comments.

Both comment-php.patch​ and export-php.patch​ are looking pretty good. One thing left:

• When you're documenting a filter, if you find yourself saying "The whatever value", in the short description, it should really be "Filter the whatever value". Yes, the apply_filters() call does in-effect evaluate to that value, but the point is that's it's a filter doc, and not just a statement of value :)

[bftrick]

Replying to DrewAPicture:

@mordauk:

In the future, please don't overwrite old patches with the new ones, makes it hard to track progress and context of comments.

How do you name the subsequent patches? foo-1.diff?

[SergeyBiryukov]

Replying to bftrick:

How do you name the subsequent patches? foo-1.diff?

Just don't check the "Replace existing attachment" checkbox. Trac will then name it foo.2.diff, etc.

[mordauk]

wp-admin/includes/comment.php

[mordauk]

wp-admin/export.php

[mordauk]

Replying to DrewAPicture:

@mordauk:

In the future, please don't overwrite old patches with the new ones, makes it hard to track progress and context of comments.

Both comment-php.patch​ and export-php.patch​ are looking pretty good. One thing left:

• When you're documenting a filter, if you find yourself saying "The whatever value", in the short description, it should really be "Filter the whatever value". Yes, the apply_filters() call does in-effect evaluate to that value, but the point is that's it's a filter doc, and not just a statement of value :)

Done, thanks for the extra feedback :)

[SergeyBiryukov]

In 25434:

Inline documentation for hooks in wp-admin/includes/comment.php.

props mordauk.
see #25229.

[SergeyBiryukov]

In 25435:

Inline documentation for hooks in wp-admin/export.php.

props mordauk.
see #25229.

[bftrick]

second pass as user-new.php

[bftrick]

Thanks for the feedback @DrewAPicture & @SergeyBiryukov! :)

[DrewAPicture]

@bftrick – user-new.2.diff​ So close!

• I'd like you to add "Default is _whatever_" for the autocomplete_users_for_site_admins and show_password_fields filter hooks, if you could. Then this one is ready to go in!

[dustyf]

First pass at wp-includes/canonical.php

[dustyf]

First pass at wp-includes/feed-rss.php

[dustyf]

Second pass at wp-includes/feed-rss.php. Forgot to wrap the doc block in php tags.

[dustyf]

Second pass at wp-includes-feed-rss2.diff - cleaned something up.

[dustyf]

First pass at wp-includes/feed-atom.php

[dustyf]

First pass at wp-includes/feed-rss2-comments.php

[dustyf]

First pass at wp-includes/feed-atom-comments.php

[dustyf]

First pass at wp-includes/feed-rdf.diff

[naomicbush]

wp-admin/plugin-install.php

[naomicbush]

wp-admin/includes/plugin-install.php

[netweb]

wp-links-opml.php 1st pass

[netweb]

Taken a shot at wp-links-opml.php

Related Tickets & Changesets #3595/r5011 & #6947/r13113

[NikV]

First Pass at wp-includes/post-thumbnail-template.php

[mordauk]

wp-includes/load.php

[mordauk]

wp-includes/load.php

[mordauk]

Oops, missed the .diff extension.

[DrewAPicture]

@mordauk:
wp-includes-load.php.diff​, just remove the empty line after the @since. The empty line only applies if there are other tags/other stuff in the doc block following it.

[mordauk]

wp-includes/load.php

[mordauk]

Replying to DrewAPicture:

@mordauk:
wp-includes-load.php.diff​, just remove the empty line after the @since. The empty line only applies if there are other tags/other stuff in the doc block following it.

Refreshed.

[DrewAPicture]

@SergeyBiryukov, @helen wp-includes-load.php.2.diff​ looks ready for commit, props @mordauk

[SergeyBiryukov]

In 25455:

Inline documentation for hooks in wp-includes/load.php.

props mordauk.
see #25229.

[bftrick]

third pass of user-new.php

[bftrick]

Replying to DrewAPicture:

@bftrick – user-new.2.diff​ So close!

• I'd like you to add "Default is _whatever_" for the autocomplete_users_for_site_admins and show_password_fields filter hooks, if you could. Then this one is ready to go in!

Alright just attached a new diff. Hopefully I understood you correctly. :)

[DrewAPicture]

Replying to bftrick:

Replying to DrewAPicture:
Alright just attached a new diff. Hopefully I understood you correctly. :)

Yep, looks good.

Edit, I'm going to make one small change. @bftrick, for future reference, any filter hook with 'pre_' prefixed on the front allows filtering a value before it is stored in the database.

[DrewAPicture]

Ready for commit

[DrewAPicture]

user-new.4.diff​ is ready for commit. Props bftrick.

[SergeyBiryukov]

pre_user_login filter was added in [3857], but is only used in the current context since [12722]. Going with @since 3.0.0.

[SergeyBiryukov]

In 25470:

Inline documentation for hooks in wp-admin/user-new.php.

props bftrick.
see #25229.

[DrewAPicture]

wp-admin-plugin-install.25229.diff​ — @naomicbush:

• Add spacing to filters and actions per coding standards while you're documenting them, e.g. apply_filters( 'hook', 'callback' ) vs apply_filters('hook', 'callback')`.
• 'install_plugins_pre_' . $tab, rather than generalizing that install or plugin information are tabs, specify that the action fires before each tab is loaded. Add a long description if you think it will make it clearer.
• Other than that, looks good.

wp-admin-includes-plugin-install.25229.diff​ — @naomicbush:

• "Requested information" is a little too succinct. Maybe be a little more descriptive.
• Space out actions and filters per coding standards
• "Merely add a result" isn't going to work. Each filter or action should be viewed as independent. "Merely add a result" makes sense in the context of the file and other hooks but not independently.
• Remove all functional docs changes. Let's just stick with hooks and filters for now. If you'd like to submit a functional docs patch separately, create a new ticket on the Inline Docs component and we'll discuss those there.

wp-links-opml.diff​ — @netweb:

• opml_head action: Space it out per coding standards, and drop the ?> to a new line. Also, the short description should describe when it fires, e.g. "Fires in the OPML head." or something
• Anytime you find yourself writing "Allows you to filter something", just use "Filter something". "Allows you to" makes an implication about how, instead of what
• @param lines 54, 69 need a period after the short description
• Spacing issues with the docblock for link_title L#64

[DrewAPicture]

wp-includes-feed-rdf.diff​ — @dustyf:

• Space out actions and filters per coding standards while you're there documenting them (it's a great opportunity), e.g. do_action( 'rdf_ns' ) vs do_action('rdf_ns')
• Change the tense of "Fired" to "Fires" in you short descriptions
• Nice work on this one! Make these changes and it's ready to go in :)

wp-includes-canonical.diff​ — @dustyf:

• Space out actions and filters per coding standards while you're there.
• There's no hard-and-fast rule for when to wrap lines, such as in a long description, but generally I try to keep it at about 10-12 words per line. So when you wrap it, the line should start with a * and a space and then the rest of the sentence. Like this: ​https://gist.github.com/DrewAPicture/5e1e395744060e50ba84
• Fix the above and I'd say it's ready to go in

wp-includes-feed-rss.2.diff​ — @dustyf:

• Change tense of "Fired" to "Fires" in both short descriptions
• That's it for this one, nice work.

wp-includes-feed-rss2.diff​ — @dustyf:

• s/Fired/Fires in short descriptions
• Space out the actions per coding standards, e.g. do_action( 'rss2_item' ); vs do_action('rss2_item');
• This goes for this patch and any future ones: Anytime you find yourself writing "Allows filtering of whatever" just change that to "Filter whatever". It's about documenting what rather than what's possible.
• @since should use the 3-digit, x.x.x style

wp-includes-feed-atom.php​ — @dustyf:

• Can you regenerate the patch in diff format?

wp-includes-feed-rss2-comments.diff​ — @dustyf:

• s/Fired/Fires
• Space out actions and filters per coding standards
• Space out the @param lines to align the types, variables and short descriptions, like this: ​https://gist.github.com/DrewAPicture/e8412b856fd7305f8ac6
• Other than that, looks good

[ericlewis]

attachment:post-thumbnail-template.php.diff​ - @NikV

• All patches should be made from either the WP root directory (svn diff wp-includes/post-thumbnail-template.php > yourfile.diff), or trunk root (svn diff src/wp-includes/post-thumbnail-template.php > yourfile.diff)
• Preferably name your patches the entire WP directory path to the file, and leave off the .php extension in the diff filename (so wp-includes/post-thumbnail-template.php would be wp-includes-post-thumbnail-template.diff)
• Parameters such as $size here should use a @see reference to a function's docblock that describe in more detail the options for $size. For instance, this can accept either a string keyword for a size, or a two-item array representing width and height in pixels. Ideally we reference the a canonical doc so as not to repeat ourselves of the options.
• "Begins to fetch the post thumbnail HTML" and "Stops fetching the post thumbnail HTML." would be better phrased as "Before fetching the post thumbnail HTML." and "After fetching the post thumbnail HTML." Actions are discrete points of execution, so using a verb like "begins" is misleading - the action is not doing anything.
• "The ID of the post where the post thumbnail will be displayed." is confusing. Here let's be brief and say "Post ID." as it does in the function docblock for get_the_post_thumbnail().
• s/iamge/image
• "The ID of a post thumbnail." should be "The post thumbnail ID." We're not talking about a generic post thumbnail, it is a specific one that has been supplied. Ditto for "The size of a post thumbnail. "
• Thanks for the work, look forward to the next draft!

[dllh]

wp-admin/includes/plugin.php

[dustyf]

Replying to DrewAPicture:

• Space out actions and filters per coding standards while you're there documenting them (it's a great opportunity), e.g. do_action( 'rdf_ns' ) vs do_action('rdf_ns')

I actually thought about this after I submitted. I'll be sure to fix it when I resubmit.

wp-includes-feed-atom.php​ — @dustyf:

• Can you regenerate the patch in diff format?

Wow, I think it was late. Not sure why I submitted this :)

I'll get the changes up before the ticket Triage tomorrow so it can get in.

[dllh]

Better patch for wp-admin/includes/plugin.php. Adds @see references where parameters come from calling functions and removes inadvertent/inaccurate parameter docs from the uninstall_$file action.

[NikV]

Replying to ericlewis:

attachment:post-thumbnail-template.php.diff​ - @NikV

• All patches should be made from either the WP root directory (svn diff wp-includes/post-thumbnail-template.php > yourfile.diff), or trunk root (svn diff src/wp-includes/post-thumbnail-template.php > yourfile.diff)
• Preferably name your patches the entire WP directory path to the file, and leave off the .php extension in the diff filename (so wp-includes/post-thumbnail-template.php would be wp-includes-post-thumbnail-template.diff)
• Parameters such as $size here should use a @see reference to a function's docblock that describe in more detail the options for $size. For instance, this can accept either a string keyword for a size, or a two-item array representing width and height in pixels. Ideally we reference the a canonical doc so as not to repeat ourselves of the options.
• "Begins to fetch the post thumbnail HTML" and "Stops fetching the post thumbnail HTML." would be better phrased as "Before fetching the post thumbnail HTML." and "After fetching the post thumbnail HTML." Actions are discrete points of execution, so using a verb like "begins" is misleading - the action is not doing anything.
• "The ID of the post where the post thumbnail will be displayed." is confusing. Here let's be brief and say "Post ID." as it does in the function docblock for get_the_post_thumbnail().
• s/iamge/image
• "The ID of a post thumbnail." should be "The post thumbnail ID." We're not talking about a generic post thumbnail, it is a specific one that has been supplied. Ditto for "The size of a post thumbnail. "
• Thanks for the work, look forward to the next draft!

How would I format the @see? Would I do it only for the $size?

[betzster]

wp-includes/class-wp-admin-bar.php

[westi]

In 25474:

Inline documentation for hooks in wp-admin/includes/plugin.php

See #25229 props dllh

[westi]

In 25475:

Inline documentation for hooks in wp-includes/class-wp-admin-bar.php

See #25229 props betzster

[netweb]

Replying to DrewAPicture:

wp-links-opml.diff​ — @netweb:

• opml_head action: Space it out per coding standards, and drop the ?> to a new line. Also, the short description should describe when it fires, e.g. "Fires in the OPML head." or something
• Anytime you find yourself writing "Allows you to filter something", just use "Filter something". "Allows you to" makes an implication about how, instead of what
• @param lines 54, 69 need a period after the short description
• Spacing issues with the docblock for link_title L#64

Thanks for the feedback :) Fixed in ​http://core.trac.wordpress.org/attachment/ticket/25229/wp-links-opml.2.diff

[NikV]

Replying to NikV:

Replying to ericlewis:

attachment:post-thumbnail-template.php.diff​ - @NikV

• All patches should be made from either the WP root directory (svn diff wp-includes/post-thumbnail-template.php > yourfile.diff), or trunk root (svn diff src/wp-includes/post-thumbnail-template.php > yourfile.diff)
• Preferably name your patches the entire WP directory path to the file, and leave off the .php extension in the diff filename (so wp-includes/post-thumbnail-template.php would be wp-includes-post-thumbnail-template.diff)
• Parameters such as $size here should use a @see reference to a function's docblock that describe in more detail the options for $size. For instance, this can accept either a string keyword for a size, or a two-item array representing width and height in pixels. Ideally we reference the a canonical doc so as not to repeat ourselves of the options.
• "Begins to fetch the post thumbnail HTML" and "Stops fetching the post thumbnail HTML." would be better phrased as "Before fetching the post thumbnail HTML." and "After fetching the post thumbnail HTML." Actions are discrete points of execution, so using a verb like "begins" is misleading - the action is not doing anything.
• "The ID of the post where the post thumbnail will be displayed." is confusing. Here let's be brief and say "Post ID." as it does in the function docblock for get_the_post_thumbnail().
• s/iamge/image
• "The ID of a post thumbnail." should be "The post thumbnail ID." We're not talking about a generic post thumbnail, it is a specific one that has been supplied. Ditto for "The size of a post thumbnail. "
• Thanks for the work, look forward to the next draft!

How would I format the @see? Would I do it only for the $size?

Never mind. Patch will be submitted soon.

[SergeyBiryukov]

wp-admin-includes-plugin.2.diff​ — @dllh:
wp-includes-class-wp-admin-bar.php.patch​ — @betzster:

It's totally fine to use @since Unknown, but if you're specifying an actual version, please make sure it's correct.

• admin_bar_init and add_admin_bar_menus actions were both introduced in 3.1 along with the admin bar itself (see [15862] and [16038]), not in 3.0.2 or 3.0.4.
• activate_* and deactivate_* were added in 2.0 (see [2697]), not in 3.0.2.
• activate_plugin, activated_plugin and their "deactivate" counterparts were added in 2.9 (see [11909]), not in 3.0.2 or 3.3.2.
• uninstall_* was added in 2.7 (see [8585]), not in 2.6.1.

Minor releases generally don't contain new hooks.

[SergeyBiryukov]

In 25477:

Correct @since for hooks in wp-admin/includes/plugin.php. see #25229.

[SergeyBiryukov]

In 25478:

Correct @since for hooks in wp-includes/class-wp-admin-bar.php. see #25229.

[SergeyBiryukov]

It's also a good idea to wait for DrewAPicture's confirmation before committing any of the patches here.

[dllh]

Replying to SergeyBiryukov:

wp-admin-includes-plugin.2.diff​ — @dllh:
It's totally fine to use @since Unknown, but if you're specifying an actual version, please make sure it's correct.

Yikes, I saw green in the blame and thought I'd found the addition, but I'll bet that was a move and not an add. Will do better next time or specify Unknown.

[a.hoereth]

wp-admin/plugin-install.php

[a.hoereth]

Ugh, forget ​wp-admin_plugin-install.php.patch. Duplicate.

[rzen]

@netweb – ​wp-links-opml.2.diff:

• If you can indent lines 33-38 (the code inside the opening <?php and closing ?> tags) I think this is all set!

@a.hoereth – ​plugin-editor.php.patch:

• Short description could instead be written as "Filter the file type extensions that are editable in the plugin editor."
• Long description becomes unnecessary with the above edit
• @since tags should use the three-digit x.x.x style, 2.8.0 in this case.

[dustyf]

Second pass at wp-includes/feed-rdf.php - Added spaces throughout the file to meet coding standards, changed tense of short comment.

[rzen]

@dustyf – ​wp-includes-feed-rdf.2.diff:

• You went above and beyond for the spacing alterations! They all look good to me, and I think this is ready to go in.
• For your other patches, though, focus your edits around the hooks themselves (rather than everything) so the patch is easier to grok and has smaller margin for errors.

[DrewAPicture]

Replying to rzen:

@dustyf – ​wp-includes-feed-rdf.2.diff:

• You went above and beyond for the spacing alterations! They all look good to me, and I think this is ready to go in.
• For your other patches, though, focus your edits around the hooks themselves (rather than everything) so the patch is easier to grok and has smaller margin for errors.

We should only be spacing out the actual action or filter lines we're documenting. Any other spacing changes will unnecessarily update the blame records for those lines.

[DrewAPicture]

Minus the extra whitespacing

[DrewAPicture]

wp-includes-feed-rdf.3.diff​ removes the extra whitespacing and is ready for commit, props @dustyf.

[SergeyBiryukov]

In 25479:

Inline documentation for hooks in wp-includes/feed-rdf.php.

props dustyf.
see #25229.

[DrewAPicture]

Fix @since versions

[DrewAPicture]

wp-links-opml.3.diff​ is ready for commit, props @netweb.

[enej]

Second pass at ms.php

[DrewAPicture]

Action fixes after [25474] and [25477]

[DrewAPicture]

wp-admin-includes-plugin.3.diff​ is ready for commit.

It has some fixes for wp-admin/includes/plugin.php following [25474] and [25477].

[dustyf]

Second Pass at wp-includes/canonical.php - Also fixes spacing on functions, hooks, etc and after ! in entire document

[SergeyBiryukov]

In 25480:

Inline documentation for hooks in wp-links-opml.php.

props netweb.
see #25229.

[DrewAPicture]

wp-admin/includes/ms.php

[DrewAPicture]

wp-admin-includes-ms.2.diff​ is ready for commit. Props @enej and @DrewAPicture.

[SergeyBiryukov]

Replying to DrewAPicture:

wp-admin-includes-plugin.3.diff​ is ready for commit.

It has some fixes for wp-admin/includes/plugin.php following [25474] and [25477].

$network_deactivating parameter for deactivate_plugin, deactivate_*, and deactivated_plugin hooks references a non-existing function (@see deactivate_plugin()). Should we correct that as well?

[DrewAPicture]

Replying to SergeyBiryukov:

Replying to DrewAPicture:

wp-admin-includes-plugin.3.diff​ is ready for commit.

It has some fixes for wp-admin/includes/plugin.php following [25474] and [25477].

$network_deactivating parameter for deactivate_plugin, deactivate_*, and deactivated_plugin hooks references a non-existing function (@see deactivate_plugin()). Should we correct that as well?

Yeah, that's probably a good idea :) Sorry, I missed that.

[rzen]

@dustyf – ​wp-includes-canonical.2.diff:

• Per ​@DrewAPicture and my comments on wp-includes-feed-rdf.2.diff, can you resubmit a patch with *only* the hook-related edits? All other edits are out-of-scope for this ticket and its patches.

[SergeyBiryukov]

In 25481:

Inline documentation for hooks in wp-admin/includes/ms.php.

props enej, DrewAPicture.
see #25229.

[SergeyBiryukov]

Hooks that come from WordPress MU should have @since MU rather than 3.0.0. I've corrected them in [25481].

[SergeyBiryukov]

Replying to DrewAPicture:

Yeah, that's probably a good idea :) Sorry, I missed that.

How does wp-admin-includes-plugin.4.diff​ look?

[DrewAPicture]

Replying to SergeyBiryukov:

Replying to DrewAPicture:

Yeah, that's probably a good idea :) Sorry, I missed that.

How does wp-admin-includes-plugin.4.diff​ look?

Sure, that works.

[SergeyBiryukov]

In 25482:

Clean up the documentation for hooks in wp-admin/includes/plugin.php.

props DrewAPicture.
see #25229.

[rzen]

Here are the results of our triage today, which wasn't much of a triage becasue DrewAPicture stayed pretty heavily on top of everything this week. Props, Drew!

26 Patches Committed:

Patch	File	Props	Changeset
nav-menu-template.2.diff​	wp-includes/nav-menu-template.php	Faison	[25410] (helen)
options-writing.patch​	wp-admin/options-writing.php	siobhyb	[25411] (helen)
shortcodes.diff​	wp-includes/shortcodes.php	natejacobs	[25423] (SergeyBiryukov)
ms-delete-site.php1.diff​	wp-admin/ms-delete-site.php	NikV	[25424] (SergeyBiryukov)
author-template.4.patch​	wp-includes/author-template.php	Frank Klein	[25429] (SergeyBiryukov)
comment-php.2.patch​	wp-admin/includes/comment.php	mordauk	[25434] (SergeyBiryukov)
export-php.2.patch​	wp-admin/export.php	mordauk	[25435] (SergeyBiryukov)
wp-includes-load.php.2.diff​	wp-includes/load.php	mordauk	[25455] (SergeyBiryukov)
user-new.4.diff​	wp-admin/user-new.php	bftrick	[25470] (SergeyBiryukov)
wp-admin-includes-plugin.2.diff​	wp-admin/includes/plugin.php	dllh	[25474] (westi), [25477] (SergeyBiryukov)
wp-includes-class-wp-admin-bar.php.patch​	wp-includes/class-wp-admin-bar.php	betzster	[25475] (westi), [25478] (SergeyBiryukov)
wp-includes-feed-rdf.3.diff​	wp-includes-feed-rdf.php	dustyf	[25479] (SergeyBiryukov)
wp-links-opml.3.diff​	wp-links-opml.php	netweb	[25480] (SergeyBiryukov)
wp-admin-includes-ms.2.diff​	wp-admin/includes/ms.php	enej, DrewAPicture	[25481] (SergeyBiryukov)
wp-admin-includes-plugin.4.diff​	wp-admin/includes/plugin.php	DrewAPicture, SergeyBiryukov	[25482] (SergeyBiryukov)
wp-admin-includes-plugin-install.25229.2.diff​	wp-admin/includes/plugin-install.php	naomicbush	[25511] (DrewAPicture)
wp-admin-plugin-install.25229.2.diff​	wp-admin/plugin-install.php	naomicbush	[25512] (DrewAPicture)
post-thumbnail-template.php.diff​	wp-includes/post-thumbnail-template.php	NikV	[25513] (DrewAPicture)
wp-includes-feed-rss.2.diff​	wp-includes/feed-rss.php	dustyf	[25529] (DrewAPicture)
wp-includes-feed-rss2.diff​	wp-includes/feed-rss2.php	dustyf	[25530] (DrewAPicture)
wp-includes-feed-atom.php​]	wp-includes/feed-atom.php	dustyf	[25531] (DrewAPicture)
wp-includes-feed-rss2-comments.diff​	wp-includes/feed-rss2-comments.php	dustyf	[25532] (DrewAPicture)
plugin-editor.php.patch​	wp-admin/plugin-editor.php	a.hoereth	[25534] (DrewAPicture)
none	wp-includes/canonical.php	dustyf	[25535] (DrewAPicture)
wp-activate.diff​	wp-activate.php	nullvariable	[25537] (DrewAPicture)
admin-ajax.diff​	wp-admin/admin-ajax.php	nullvariable	[25538] (DrewAPicture)

Patches Awaiting Edits:
None''

Patches Awaiting Review:
None!

Patches Ready for Commit:
None

Workflow Notes
During the inline-docs meeting today we discussed that, going forward, all new files will be handled via their own tickets, rather than tracked in this single massive ticket. I'll be updating the ​make/core post to reflect the new workflow steps, so please look there for further instruction.

Those of you with patches attached to this ticket (naomicbush, dustyf, NikV, a.hoereth) will continue to submit those patches here. All new files will become their own tickets and be patched there.

Thanks everyone for your contributions so far!

[jamescollins]

Minor fixes/changes to [25481]

[jamescollins]

Replying to SergeyBiryukov:

Hooks that come from WordPress MU should have @since MU rather than 3.0.0. I've corrected them in [25481].

wp-admin-includes-ms.3.diff​ improves on [25481] by making it clear whether the hooks fire before of after the related event occurs.

[SergeyBiryukov]

In 25486:

Update inline documentation for hooks in wp-admin/includes/ms.php.

props jamescollins.
see #25229.

[SergeyBiryukov]

Some more fixes in wp-admin-includes-ms.4.diff​ ("User ID of the user" seemed a bit redundant).

[SergeyBiryukov]

In 25497:

Some more tweaks for the documentation in wp-admin/includes/ms.php. see #25229.

[naomicbush]

wp-admin/includes/plugin-install.php

[naomicbush]

wp-admin/plugin-install.php

[naomicbush]

Replying to DrewAPicture:

wp-admin-plugin-install.25229.diff​ — @naomicbush:

• Add spacing to filters and actions per coding standards while you're documenting them, e.g. apply_filters( 'hook', 'callback' ) vs apply_filters('hook', 'callback')`.
• 'install_plugins_pre_' . $tab, rather than generalizing that install or plugin information are tabs, specify that the action fires before each tab is loaded. Add a long description if you think it will make it clearer.
• Other than that, looks good.

wp-admin-includes-plugin-install.25229.diff​ — @naomicbush:

• "Requested information" is a little too succinct. Maybe be a little more descriptive.
• Space out actions and filters per coding standards
• "Merely add a result" isn't going to work. Each filter or action should be viewed as independent. "Merely add a result" makes sense in the context of the file and other hooks but not independently.
• Remove all functional docs changes. Let's just stick with hooks and filters for now. If you'd like to submit a functional docs patch separately, create a new ticket on the Inline Docs component and we'll discuss those there.

Thanks for the feedback, DrewAPicture. Patches revised.

[NikV]

Second Pass at wp-includes/post-thumbnail-template.php. Thanks @ericlewis

[DrewAPicture]

Replying to onalioa:

Please ask your question on the ​WordPress Support Forums. Trac is for core development.

[DrewAPicture]

In 25511:

Inline documentation for hooks in wp-admin/includes/plugin-install.php.

Props naomicbush.
See #25229.

[DrewAPicture]

In 25512:

Inline documentation for hooks in wp-admin/plugin-install.php.

Props naomicbush.
See #25229.

[DrewAPicture]

In 25513:

Inline documentation for hooks in wp-includes/post-thumbnail-template.php.

Props NikV.
See #25229.

[DrewAPicture]

In 25529:

Inline documentation for hooks in wp-includes/feed-rss.php.

Props dustyf.
See #25229.

[DrewAPicture]

In 25530:

Inline documentation for hooks in wp-includes/feed-rss2.php.

Props dustyf for the initial patch.
See #25229.

[DrewAPicture]

In 25531:

Inline documentation for hooks in wp-includes/feed-atom.php.

Props dustyf.
See #25229.

[DrewAPicture]

In 25532:

Inline documentation of hooks for wp-includes/feed-rss2-comments.php.

Props dustyf.
See #25229.

[DrewAPicture]

In 25534:

Inline documentation for hooks in wp-admin/plugin-editor.php.

Props a.hoereth.
See #25229.

[DrewAPicture]

In 25535:

Inline documentation for hooks in wp-includes/canonical.php.

Props dustyf for the initial patch.
See #25229.

[DrewAPicture]

I think we've wrapped up all outstanding patches here.

See comment:38 and comment:106 for the lists of committed patches and contributors.

Please create new tickets for new files per the new workflow outlined on the ​make/core post.

[DrewAPicture]

Looks like a couple of patches were missed. Both were reviewed in comment:38:

• wp-activate.diff​ — @nullvariable
• admin-ajax.diff​ — @nullvariable

[DrewAPicture]

In 25537:

Inline documentation for hooks in wp-activate.php.

Props nullvariable for the initial patch.
See #25229.

[DrewAPicture]

In 25538:

Inline documentation for hooks in wp-admin/admin-ajax.php.

Props nullvariable for the initial patch.
Fixes #25229.

[DrewAPicture]

In 25539:

Fix spaces to tabs and inline documenation formatting in wp-admin/admin-ajax.php.

See #25229.

[nullvariable]

Replying to DrewAPicture:

Looks like a couple of patches were missed. Both were reviewed in comment:38:

• wp-activate.diff​ — @nullvariable
• admin-ajax.diff​ — @nullvariable

eh my bad! Just saw that.

[DrewAPicture]

Replying to nullvariable:

Replying to DrewAPicture:

Looks like a couple of patches were missed. Both were reviewed in comment:38:

• wp-activate.diff​ — @nullvariable
• admin-ajax.diff​ — @nullvariable

eh my bad! Just saw that.

No sweat, we got them in. Thanks for the patches, hope to see more from you in separate tickets :)

[dd32]

In 26509:

Add braces around a conditional hook. This wasn't causing an issue as if ( conditional ) /* multiline comment */ command(); is perfectly OK, but left open doubt and potential future bugs. See #25229

[DrewAPicture]

In 27144:

Fixes for inline documentation for hooks in wp-comments-post.php.

Adds missing @since versions, spacing, and language tweaks.

See #26869, #25229, [25249].

[DrewAPicture]

In 27145:

Fixes for hooks documentation in wp-admin/admin-footer.php.

Adds some new lines, some hook-line spacing, docs-specific variables, and other language tweaks.

See #26869, #25229, [25252].

[DrewAPicture]

In 27146:

Fixes for hooks documentation on xmlrpc.php.

Adds spacing, changes an @see to @link, adds a docs-specific variable for a parameter.

See #26869, #25229 and [25281].

[DrewAPicture]

In 27147:

Fixes for hooks documentation in wp-includes/wp-db.php.

See #26869, #25229 and [25284].

[DrewAPicture]

In 27201:

Fixes for hooks documentation wp-includes/nav-menu-template.php.

See #26869, #25229 and [25410].

[DrewAPicture]

In 28833:

Add braces missed while adding docs for the option_page_capability_{$option_page} hook.

See [25372]. See #25229.