WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 21 months ago

Last modified 11 months ago

#25229 closed task (blessed) (fixed)

Add Inline Docs for Hooks

Reported by: rzen Owned by: DrewAPicture
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Inline Docs Keywords:
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Document ALL THE HOOKS!
http://d.pr/i/UMY6+

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.

Attachments (73)

wp-comments-post.diff (3.5 KB) - added by rzen 21 months ago.
Second pass at ./wp-comments-post.dff
admin-footer.diff (2.2 KB) - added by natejacobs 21 months ago.
First pass at wp-admin/admin-footer.php
index.diff (836 bytes) - added by natejacobs 21 months ago.
First pass at wp-admin/index.php
wp-trackback.diff (579 bytes) - added by bananastalktome 21 months ago.
Pass at wp-trackback.php (not 100% on the @since, but pretty sure)
xmlrpc.diff (1.2 KB) - added by bftrick 21 months ago.
first pass at xmlrpc.php
wp-comment.diff (721 bytes) - added by dustyf 21 months ago.
first pass at wp-admin/comment.php
custom-background.diff (1.5 KB) - added by dustyf 21 months ago.
First pass at wp-admin/custom-background.php
wp-db.diff (701 bytes) - added by natejacobs 21 months ago.
wp-includes/wp-db.php
comments.diff (754 bytes) - added by natejacobs 21 months ago.
wp-includes/theme-compat/comments.php
comments-popup.diff (952 bytes) - added by natejacobs 21 months ago.
wp-includes/theme-compat/comments.php
rss.diff (479 bytes) - added by natejacobs 21 months ago.
wp-includes/rss.php
nav-menu-template.diff (5.4 KB) - added by Faison 21 months ago.
First pass at wp-includes/nav-menu-template.php, questions to follow.
nav-menu-template.2.diff (5.4 KB) - added by Faison 21 months ago.
Second Pass, forgot to add description for walker_nav_menu_start_el's first parameter
author-template.patch (14.2 KB) - added by Frank Klein 21 months ago.
Adds filter documentation for wp-includes/author-template.php. Also updates outdated function documentation.
ms.diff (9.3 KB) - added by enej 21 months ago.
First pass at ms.php
http.php.diff (1.8 KB) - added by tw2113 21 months ago.
http.php.1.diff (1.8 KB) - added by tw2113 21 months ago.
Fixes some inconsistencies noticed by DrewAPicture
http.php.2.diff (1.8 KB) - added by tw2113 21 months ago.
I'll get this right eventually :)
http.php.3.diff (2.0 KB) - added by tw2113 21 months ago.
last one
http.php.4.diff (2.3 KB) - added by tw2113 21 months ago.
Capitalization consistency for URL and HTTP. Includes a couple non-hook comments.
wp-activate.diff (1.0 KB) - added by nullvariable 21 months ago.
wp-activate.php inline hook documentation, and minor corrections on version for other documentation.
ms-delete-site.php.diff (705 bytes) - added by NikV 21 months ago.
First pass at wp-admin/ms-delete-site.php
admin-ajax.diff (1.3 KB) - added by nullvariable 21 months ago.
inline documentation for admin-ajax.php.
options.patch (1.5 KB) - added by siobhyb 21 months ago.
Second pass at options.php. Thanks @DrewAPicture, @tierra, and @kimparsell.
options-writing.patch (2.4 KB) - added by siobhyb 21 months ago.
First pass at wp-admin/options-writing.php. 3 of the 4 hooks here are duplicates.
shortcodes.diff (819 bytes) - added by natejacobs 21 months ago.
Second pass of wp-includes/shortcodes.php. Thanks @drewapicture
ms-delete-site.php1.diff (1.1 KB) - added by NikV 21 months ago.
Second Pass at wp-admin/ms-delete-site.php. Thanks @DrewAPicture.
user-new.diff (2.8 KB) - added by bftrick 21 months ago.
adding 5 doc blocks to user-new.php
author-template.2.patch (2.9 KB) - added by Frank Klein 21 months ago.
Updated patch containing only inline documentation changes.
author-template.3.patch (3.0 KB) - added by Frank Klein 21 months ago.
Updated patch according to comment:44
author-template.4.patch (3.4 KB) - added by DrewAPicture 21 months ago.
comment-php.patch (677 bytes) - added by mordauk 21 months ago.
wp-admin/includes/comment.php
export-php.patch (1.1 KB) - added by mordauk 21 months ago.
wp-admin/export.php
comment-php.2.patch (682 bytes) - added by mordauk 21 months ago.
wp-admin/includes/comment.php
export-php.2.patch (1.1 KB) - added by mordauk 21 months ago.
wp-admin/export.php
user-new.2.diff (2.8 KB) - added by bftrick 21 months ago.
second pass as user-new.php
wp-includes-canonical.diff (1.4 KB) - added by dustyf 21 months ago.
First pass at wp-includes/canonical.php
wp-includes-feed-rss.diff (926 bytes) - added by dustyf 21 months ago.
First pass at wp-includes/feed-rss.php
wp-includes-feed-rss.2.diff (955 bytes) - added by dustyf 21 months ago.
Second pass at wp-includes/feed-rss.php. Forgot to wrap the doc block in php tags.
wp-includes-feed-rss2.diff (2.1 KB) - added by dustyf 21 months ago.
Second pass at wp-includes-feed-rss2.diff - cleaned something up.
wp-includes-feed-atom.php (2.2 KB) - added by dustyf 21 months ago.
First pass at wp-includes/feed-atom.php
wp-includes-feed-rss2-comments.diff (2.5 KB) - added by dustyf 21 months ago.
First pass at wp-includes/feed-rss2-comments.php
wp-includes-feed-atom-comments.diff (2.1 KB) - added by dustyf 21 months ago.
First pass at wp-includes/feed-atom-comments.php
wp-includes-feed-rdf.diff (1.7 KB) - added by dustyf 21 months ago.
First pass at wp-includes/feed-rdf.diff
wp-admin-plugin-install.25229.diff (1.6 KB) - added by naomicbush 21 months ago.
wp-admin/plugin-install.php
wp-admin-includes-plugin-install.25229.diff (4.8 KB) - added by naomicbush 21 months ago.
wp-admin/includes/plugin-install.php
wp-links-opml.diff (1.6 KB) - added by netweb 21 months ago.
wp-links-opml.php 1st pass
post-thumbnail-template.php.diff (2.2 KB) - added by NikV 21 months ago.
First Pass at wp-includes/post-thumbnail-template.php
wp-includes-load.php (395 bytes) - added by mordauk 21 months ago.
wp-includes/load.php
wp-includes-load.php.diff (395 bytes) - added by mordauk 21 months ago.
wp-includes/load.php
wp-includes-load.php.2.diff (390 bytes) - added by mordauk 21 months ago.
wp-includes/load.php
user-new.3.diff (2.8 KB) - added by bftrick 21 months ago.
third pass of user-new.php
user-new.4.diff (3.1 KB) - added by DrewAPicture 21 months ago.
Ready for commit
wp-admin-includes-plugin.diff (4.1 KB) - added by dllh 21 months ago.
wp-admin/includes/plugin.php
wp-admin-includes-plugin.2.diff (3.7 KB) - added by dllh 21 months ago.
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.
wp-includes-class-wp-admin-bar.php.patch (933 bytes) - added by betzster 21 months ago.
wp-includes/class-wp-admin-bar.php
wp-links-opml.2.diff (1.6 KB) - added by netweb 21 months ago.
plugin-editor.php.patch (817 bytes) - added by a.hoereth 21 months ago.
wp-admin_plugin-install.php.patch (933 bytes) - added by a.hoereth 21 months ago.
wp-admin/plugin-install.php
wp-includes-feed-rdf.2.diff (3.5 KB) - added by dustyf 21 months ago.
Second pass at wp-includes/feed-rdf.php - Added spaces throughout the file to meet coding standards, changed tense of short comment.
wp-includes-feed-rdf.3.diff (1.7 KB) - added by DrewAPicture 21 months ago.
Minus the extra whitespacing
wp-links-opml.3.diff (1.7 KB) - added by DrewAPicture 21 months ago.
Fix @since versions
ms.2.diff (10.4 KB) - added by enej 21 months ago.
Second pass at ms.php
wp-admin-includes-plugin.3.diff (2.6 KB) - added by DrewAPicture 21 months ago.
Action fixes after [25474] and [25477]
wp-includes-canonical.2.diff (29.2 KB) - added by dustyf 21 months ago.
Second Pass at wp-includes/canonical.php - Also fixes spacing on functions, hooks, etc and after ! in entire document
wp-admin-includes-ms.diff (10.2 KB) - added by DrewAPicture 21 months ago.
wp-admin/includes/ms.php
wp-admin-includes-ms.2.diff (10.2 KB) - added by DrewAPicture 21 months ago.
wp-admin-includes-plugin.4.diff (5.0 KB) - added by SergeyBiryukov 21 months ago.
wp-admin-includes-ms.3.diff (1.7 KB) - added by jamescollins 21 months ago.
Minor fixes/changes to [25481]
wp-admin-includes-ms.4.diff (3.5 KB) - added by SergeyBiryukov 21 months ago.
wp-admin-includes-plugin-install.25229.2.diff (2.0 KB) - added by naomicbush 21 months ago.
wp-admin/includes/plugin-install.php
wp-admin-plugin-install.25229.2.diff (1.7 KB) - added by naomicbush 21 months ago.
wp-admin/plugin-install.php
post-thumbnail-template1.diff (2.1 KB) - added by NikV 21 months ago.
Second Pass at wp-includes/post-thumbnail-template.php. Thanks @ericlewis

Download all attachments as: .zip

Change History (209)

comment:1 @tw211321 months ago

  • Cc michael.d.beckwith@… added

comment:2 @nacin21 months ago

  • Milestone changed from Awaiting Review to 3.7
  • Type changed from enhancement to task (blessed)

comment:3 @kevinlangleyjr21 months ago

  • Cc kevinlangleyjr added

@rzen21 months ago

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

comment:4 @DrewAPicture21 months ago

  • Cc xoodrew@… added; DrewAPicture removed

comment:5 @DrewAPicture21 months ago

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

comment:6 @SergeyBiryukov21 months ago

  • Description modified (diff)

comment:7 @SergeyBiryukov21 months ago

  • Description modified (diff)

@natejacobs21 months ago

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

@natejacobs21 months ago

First pass at wp-admin/index.php

@bananastalktome21 months ago

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

comment:8 @ev3rywh3re21 months ago

  • Cc jess@… added

comment:9 @nacin21 months ago

In 25249:

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

props rzen.
see #25229.

comment:10 @nacin21 months ago

In 25250:

Inline documentation for the welcome_panel hook.

props natejacobs.
see #25229.

comment:11 @nacin21 months ago

In 25251:

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

comment:12 @nacin21 months ago

In 25252:

Hook docs for admin-footer.php.

props natejacobs.
see #25229.

comment:13 @nacin21 months ago

In 25253:

Document the trackback_post hook in wp-trackback.php.

props bananastalktome.
see #25229.

comment:14 @nacin21 months ago

In 25273:

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

@bftrick21 months ago

first pass at xmlrpc.php

@dustyf21 months ago

first pass at wp-admin/comment.php

@dustyf21 months ago

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

@natejacobs21 months ago

wp-includes/wp-db.php

@natejacobs21 months ago

wp-includes/theme-compat/comments.php

@natejacobs21 months ago

wp-includes/theme-compat/comments.php

comment:15 follow-up: @natejacobs21 months ago

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?

@natejacobs21 months ago

wp-includes/rss.php

comment:16 in reply to: ↑ 15 @nacin21 months ago

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.

comment:17 @nacin21 months ago

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

comment:18 @nacin21 months ago

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.

comment:19 @nacin21 months ago

In 25282:

Document comment_edit_redirect.

props dustyf.
see #25229.

comment:20 @nacin21 months ago

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.

comment:21 @nacin21 months ago

In 25284:

Document the 'query' filter in wp-db.

props natejacobs.
see #25229.

comment:22 @nacin21 months ago

In 25286:

Inline docs for hooks in MagPie.

props natejacobs.
see #25229.

comment:23 @nacin21 months ago

In 25288:

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

@Faison21 months ago

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

comment:24 @Faison21 months ago

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

@Faison21 months ago

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

comment:25 @Alphawolf21 months ago

  • Cc scripts@… added

comment:26 @nacin21 months ago

In 25290:

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

@Frank Klein21 months ago

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

comment:27 follow-up: @dustyf21 months ago

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?

Last edited 21 months ago by dustyf (previous) (diff)

comment:28 in reply to: ↑ 27 @DrewAPicture21 months ago

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.

@enej21 months ago

First pass at ms.php

comment:29 @DrewAPicture21 months ago

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

comment:30 @tw211321 months ago

wp-includes/http.php done.

@tw211321 months ago

@tw211321 months ago

Fixes some inconsistencies noticed by DrewAPicture

@tw211321 months ago

I'll get this right eventually :)

comment:31 @DrewAPicture21 months ago

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.

@tw211321 months ago

last one

@tw211321 months ago

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

comment:32 @SergeyBiryukov21 months ago

In 25302:

Inline documentation for hooks in http.php.

props tw2113.
see #25229.

@nullvariable21 months ago

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

@NikV21 months ago

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

@nullvariable21 months ago

inline documentation for admin-ajax.php.

@siobhyb21 months ago

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

comment:33 @DrewAPicture21 months ago

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

@siobhyb21 months ago

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

@natejacobs21 months ago

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

comment:34 follow-up: @bftrick21 months ago

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' );
}

comment:35 in reply to: ↑ 34 ; follow-up: @rmccue21 months ago

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.

comment:36 @SergeyBiryukov21 months ago

In 25372:

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

props siobhyb.
see #25229.

comment:37 in reply to: ↑ 35 @bftrick21 months ago

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! :)

comment:38 @DrewAPicture21 months ago

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.diffwp-comments-post.php[25249], [25251]
admin-footer.diffwp-admin/admin-footer.php[25252]
index.diffwp-admin/index.php[25250]
wp-trackback.diffwp-trackback[25253]
xmlrpc.diffxmlrpc.php[25281]
wp-comment.diffwp-admin/comment.php[25282]
custom-background.diffwp-admin/custom-background.php[25283]
wp-db.diffwp-includes/wp-db.php[25284]
rss.diffwp-includes/rss.php[25286]
http.php.4.diffwp-includes/http.php[25302]
options.patchwp-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.patchwp-admin/options-writing.phpsiobhyb[25411]Committed by helen
nav-menu-template.2.diffwp-includes/nav-menu-template.phpFaison[25410]Please add a period to the short description for the wp_nav_menu filter Committed by helen
shortcodes.diffwp-includes/shortcodes.phpnatejacobs[25423]Committed by SergeyBiryukov
ms-delete-site.php1.diffwp-admin/ms-delete-site.phpNikV[25424]Committed by SergeyBiryukov
author-template.4.patchwp-includes/author-template.phpFrank Klein[25429]Committed by SergeyBiryukov
comment-php.2.patchwp-admin/includes/comment.phpmordauk[25434]Committed by SergeyBiryukov
export-php.2.patchwp-admin/export.phpmordauk[25435]Committed by SergeyBiryukov
wp-includes-load.php.2.diffwp-includes/load.phpmordauk[25455]Committed by SergeyBiryukov
user-new.4.diffwp-admin/user-new.phpbftrick[25470]Committed by SergeyBiryukov
wp-includes-feed-rdf.3.diffwp-includes/feed-rdf.phpdustyf[25479]Committed by SergeyBiryukov
wp-admin-includes-plugin.2.diffwp-admin/includes/plugin.phpdllh[25474]Committed by westi
wp-includes-class-wp-admin-bar.php.patchwp-includes/class-wp-admin-bar.phpbetzster[25475]Committed by westi
wp-links-opml.3.diffwp-links-opml.phpnetweb[25480]Committed by SergeyBiryukov
wp-admin-includes-ms.2.diffwp-admin/includes/ms.phpenej, DrewAPictureReady for commit
Last edited 21 months ago by DrewAPicture (previous) (diff)

comment:39 @helen21 months ago

In 25410:

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

comment:40 @helen21 months ago

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

comment:41 @helen21 months ago

In 25411:

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

comment:42 @helen21 months ago

In 25412:

No space between and duplicate_hook. see #25229

@NikV21 months ago

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

@bftrick21 months ago

adding 5 doc blocks to user-new.php

@Frank Klein21 months ago

Updated patch containing only inline documentation changes.

comment:43 @DrewAPicture21 months ago

Edit: @SergeyBiryukov took care of it on commit.

Last edited 21 months ago by DrewAPicture (previous) (diff)

comment:44 @DrewAPicture21 months ago

@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 :)

comment:45 @DrewAPicture21 months ago

@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.

comment:46 @SergeyBiryukov21 months ago

In 25423:

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

props natejacobs.
see #25229.

comment:47 @SergeyBiryukov21 months ago

In 25424:

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

props NikV.
see #25229.

@Frank Klein21 months ago

Updated patch according to comment:44

comment:48 @mordauk21 months ago

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

comment:49 follow-up: @DrewAPicture21 months ago

@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.

comment:50 @DrewAPicture21 months ago

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

@mordauk21 months ago

wp-admin/includes/comment.php

@mordauk21 months ago

wp-admin/export.php

comment:51 in reply to: ↑ 49 @mordauk21 months ago

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!

comment:52 @SergeyBiryukov21 months ago

In 25429:

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

props Frank Klein.
see #25229.

comment:53 follow-ups: @DrewAPicture21 months ago

@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 :)
Last edited 21 months ago by DrewAPicture (previous) (diff)

comment:54 in reply to: ↑ 53 ; follow-up: @bftrick21 months ago

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?

comment:55 in reply to: ↑ 54 @SergeyBiryukov21 months ago

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.

@mordauk21 months ago

wp-admin/includes/comment.php

@mordauk21 months ago

wp-admin/export.php

comment:56 in reply to: ↑ 53 @mordauk21 months ago

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 :)

comment:57 @SergeyBiryukov21 months ago

In 25434:

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

props mordauk.
see #25229.

comment:58 follow-up: @SergeyBiryukov21 months ago

In 25435:

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

props mordauk.
see #25229.

@bftrick21 months ago

second pass as user-new.php

comment:59 in reply to: ↑ 58 @bftrick21 months ago

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

comment:60 follow-up: @DrewAPicture21 months ago

@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!

@dustyf21 months ago

First pass at wp-includes/canonical.php

@dustyf21 months ago

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

@dustyf21 months ago

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

@dustyf21 months ago

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

@dustyf21 months ago

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

@dustyf21 months ago

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

@dustyf21 months ago

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

@dustyf21 months ago

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

@naomicbush21 months ago

wp-admin/plugin-install.php

@naomicbush21 months ago

wp-admin/includes/plugin-install.php

@netweb21 months ago

wp-links-opml.php 1st pass

comment:61 @netweb21 months ago

Taken a shot at wp-links-opml.php

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

comment:62 @netweb21 months ago

  • Cc netweb added

comment:63 @Frank Klein21 months ago

  • Cc contact@… added

@NikV21 months ago

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

@mordauk21 months ago

wp-includes/load.php

@mordauk21 months ago

wp-includes/load.php

comment:64 @mordauk21 months ago

Oops, missed the .diff extension.

comment:65 follow-up: @DrewAPicture21 months ago

@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.

@mordauk21 months ago

wp-includes/load.php

comment:66 in reply to: ↑ 65 @mordauk21 months ago

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.

comment:67 @DrewAPicture21 months ago

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

comment:68 @SergeyBiryukov21 months ago

In 25455:

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

props mordauk.
see #25229.

@bftrick21 months ago

third pass of user-new.php

comment:69 in reply to: ↑ 60 ; follow-up: @bftrick21 months ago

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. :)

comment:70 in reply to: ↑ 69 @DrewAPicture21 months ago

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.

Last edited 21 months ago by DrewAPicture (previous) (diff)

@DrewAPicture21 months ago

Ready for commit

comment:71 @DrewAPicture21 months ago

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

comment:72 @SergeyBiryukov21 months ago

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

comment:73 @SergeyBiryukov21 months ago

In 25470:

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

props bftrick.
see #25229.

comment:74 follow-ups: @DrewAPicture21 months ago

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

comment:75 follow-up: @DrewAPicture21 months ago

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:

comment:76 follow-up: @ericlewis21 months ago

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!

@dllh21 months ago

wp-admin/includes/plugin.php

comment:77 in reply to: ↑ 75 @dustyf21 months ago

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.

@dllh21 months ago

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.

comment:78 in reply to: ↑ 76 ; follow-up: @NikV21 months ago

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?

Last edited 21 months ago by NikV (previous) (diff)

@betzster21 months ago

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

comment:79 @westi21 months ago

In 25474:

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

See #25229 props dllh

comment:80 @westi21 months ago

In 25475:

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

See #25229 props betzster

@netweb21 months ago

comment:81 in reply to: ↑ 74 @netweb21 months ago

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

comment:82 in reply to: ↑ 78 @NikV21 months ago

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.

comment:83 follow-up: @SergeyBiryukov21 months ago

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.

comment:84 @SergeyBiryukov21 months ago

In 25477:

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

comment:85 @SergeyBiryukov21 months ago

In 25478:

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

comment:86 @SergeyBiryukov21 months ago

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

comment:87 in reply to: ↑ 83 @dllh21 months ago

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.hoereth21 months ago

wp-admin/plugin-install.php

comment:89 @rzen21 months ago

@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.

@dustyf21 months ago

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

comment:90 follow-up: @rzen21 months ago

@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.

comment:91 in reply to: ↑ 90 @DrewAPicture21 months ago

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.

@DrewAPicture21 months ago

Minus the extra whitespacing

comment:92 @DrewAPicture21 months ago

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

comment:93 @SergeyBiryukov21 months ago

In 25479:

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

props dustyf.
see #25229.

@DrewAPicture21 months ago

Fix @since versions

comment:94 @DrewAPicture21 months ago

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

@enej21 months ago

Second pass at ms.php

@DrewAPicture21 months ago

Action fixes after [25474] and [25477]

comment:95 follow-up: @DrewAPicture21 months ago

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

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

@dustyf21 months ago

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

comment:96 @SergeyBiryukov21 months ago

In 25480:

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

props netweb.
see #25229.

@DrewAPicture21 months ago

wp-admin/includes/ms.php

comment:97 @DrewAPicture21 months ago

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

comment:98 in reply to: ↑ 95 ; follow-up: @SergeyBiryukov21 months ago

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?

comment:99 in reply to: ↑ 98 ; follow-up: @DrewAPicture21 months ago

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.

comment:100 @rzen21 months ago

@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.

comment:101 @SergeyBiryukov21 months ago

In 25481:

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

props enej, DrewAPicture.
see #25229.

comment:102 follow-up: @SergeyBiryukov21 months ago

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

comment:103 in reply to: ↑ 99 ; follow-up: @SergeyBiryukov21 months ago

Replying to DrewAPicture:

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

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

comment:104 in reply to: ↑ 103 @DrewAPicture21 months ago

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.

comment:105 @SergeyBiryukov21 months ago

In 25482:

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

props DrewAPicture.
see #25229.

comment:106 @rzen21 months ago

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!

Last edited 21 months ago by DrewAPicture (previous) (diff)

@jamescollins21 months ago

Minor fixes/changes to [25481]

comment:107 in reply to: ↑ 102 @jamescollins21 months ago

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.

comment:108 @SergeyBiryukov21 months ago

In 25486:

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

props jamescollins.
see #25229.

comment:109 @SergeyBiryukov21 months ago

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

comment:110 @SergeyBiryukov21 months ago

In 25497:

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

@naomicbush21 months ago

wp-admin/includes/plugin-install.php

@naomicbush21 months ago

wp-admin/plugin-install.php

comment:111 in reply to: ↑ 74 @naomicbush21 months ago

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.

@NikV21 months ago

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

comment:112 follow-up: @onalioa21 months ago

Last edited 21 months ago by DrewAPicture (previous) (diff)

comment:113 in reply to: ↑ 112 @DrewAPicture21 months ago

Replying to onalioa:

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

comment:114 @DrewAPicture21 months ago

In 25511:

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

Props naomicbush.
See #25229.

comment:115 @DrewAPicture21 months ago

In 25512:

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

Props naomicbush.
See #25229.

comment:116 @DrewAPicture21 months ago

In 25513:

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

Props NikV.
See #25229.

comment:117 @DrewAPicture21 months ago

In 25529:

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

Props dustyf.
See #25229.

comment:118 @DrewAPicture21 months ago

In 25530:

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

Props dustyf for the initial patch.
See #25229.

comment:119 @DrewAPicture21 months ago

In 25531:

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

Props dustyf.
See #25229.

comment:120 @DrewAPicture21 months ago

In 25532:

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

Props dustyf.
See #25229.

comment:121 @DrewAPicture21 months ago

In 25534:

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

Props a.hoereth.
See #25229.

comment:122 @DrewAPicture21 months ago

In 25535:

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

Props dustyf for the initial patch.
See #25229.

comment:123 @DrewAPicture21 months ago

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

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.

Last edited 21 months ago by DrewAPicture (previous) (diff)

comment:124 follow-up: @DrewAPicture21 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:125 @DrewAPicture21 months ago

In 25537:

Inline documentation for hooks in wp-activate.php.

Props nullvariable for the initial patch.
See #25229.

comment:126 @DrewAPicture21 months ago

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

In 25538:

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

Props nullvariable for the initial patch.
Fixes #25229.

comment:127 @DrewAPicture21 months ago

In 25539:

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

See #25229.

comment:128 in reply to: ↑ 124 ; follow-up: @nullvariable20 months ago

Replying to DrewAPicture:

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

eh my bad! Just saw that.

comment:129 in reply to: ↑ 128 @DrewAPicture20 months ago

Replying to nullvariable:

Replying to DrewAPicture:

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

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 :)

comment:130 @dd3218 months ago

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

comment:131 @DrewAPicture16 months ago

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].

comment:132 @DrewAPicture16 months ago

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].

comment:133 @DrewAPicture16 months ago

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].

comment:134 @DrewAPicture16 months ago

In 27147:

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

See #26869, #25229 and [25284].

comment:135 @DrewAPicture15 months ago

In 27201:

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

See #26869, #25229 and [25410].

comment:136 @DrewAPicture11 months ago

In 28833:

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

See [25372]. See #25229.

Note: See TracTickets for help on using tickets.