Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 6 years ago

#25229 closed task (blessed) (fixed)

Add Inline Docs for Hooks

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

Download all attachments as: .zip

Change History (209)

#1 @tw2113
11 years ago

  • Cc michael.d.beckwith@… added

#2 @nacin
11 years ago

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

#3 @kevinlangleyjr
11 years ago

  • Cc kevinlangleyjr added

@rzen
11 years ago

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

#4 @DrewAPicture
11 years ago

  • Cc xoodrew@… added; DrewAPicture removed

#5 @DrewAPicture
11 years ago

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

#6 @SergeyBiryukov
11 years ago

  • Description modified (diff)

#7 @SergeyBiryukov
11 years ago

  • Description modified (diff)

@natejacobs
11 years ago

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

@natejacobs
11 years ago

First pass at wp-admin/index.php

@bananastalktome
11 years ago

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

#8 @ev3rywh3re
11 years ago

  • Cc jess@… added

#9 @nacin
11 years ago

In 25249:

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

props rzen.
see #25229.

#10 @nacin
11 years ago

In 25250:

Inline documentation for the welcome_panel hook.

props natejacobs.
see #25229.

#11 @nacin
11 years ago

In 25251:

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

#12 @nacin
11 years ago

In 25252:

Hook docs for admin-footer.php.

props natejacobs.
see #25229.

#13 @nacin
11 years ago

In 25253:

Document the trackback_post hook in wp-trackback.php.

props bananastalktome.
see #25229.

#14 @nacin
11 years ago

In 25273:

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

@bftrick
11 years ago

first pass at xmlrpc.php

@dustyf
11 years ago

first pass at wp-admin/comment.php

@dustyf
11 years ago

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

@natejacobs
11 years ago

wp-includes/wp-db.php

@natejacobs
11 years ago

wp-includes/theme-compat/comments.php

@natejacobs
11 years ago

wp-includes/theme-compat/comments.php

#15 follow-up: @natejacobs
11 years 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?

@natejacobs
11 years ago

wp-includes/rss.php

#16 in reply to: ↑ 15 @nacin
11 years 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.

#17 @nacin
11 years ago

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

#18 @nacin
11 years 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.

#19 @nacin
11 years ago

In 25282:

Document comment_edit_redirect.

props dustyf.
see #25229.

#20 @nacin
11 years 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.

#21 @nacin
11 years ago

In 25284:

Document the 'query' filter in wp-db.

props natejacobs.
see #25229.

#22 @nacin
11 years ago

In 25286:

Inline docs for hooks in MagPie.

props natejacobs.
see #25229.

#23 @nacin
11 years ago

In 25288:

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

@Faison
11 years ago

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

#24 @Faison
11 years 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

@Faison
11 years ago

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

#25 @Alphawolf
11 years ago

  • Cc scripts@… added

#26 @nacin
11 years ago

In 25290:

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

@Frank Klein
11 years ago

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

#27 follow-up: @dustyf
11 years 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 11 years ago by dustyf (previous) (diff)

#28 in reply to: ↑ 27 @DrewAPicture
11 years 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.

@enej
11 years ago

First pass at ms.php

#29 @DrewAPicture
11 years ago

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

#30 @tw2113
11 years ago

wp-includes/http.php done.

@tw2113
11 years ago

@tw2113
11 years ago

Fixes some inconsistencies noticed by DrewAPicture

@tw2113
11 years ago

I'll get this right eventually :)

#31 @DrewAPicture
11 years 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.

@tw2113
11 years ago

last one

@tw2113
11 years ago

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

#32 @SergeyBiryukov
11 years ago

In 25302:

Inline documentation for hooks in http.php.

props tw2113.
see #25229.

@nullvariable
11 years ago

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

@NikV
11 years ago

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

@nullvariable
11 years ago

inline documentation for admin-ajax.php.

@siobhyb
11 years ago

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

#33 @DrewAPicture
11 years ago

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

@siobhyb
11 years ago

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

@natejacobs
11 years ago

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

#34 follow-up: @bftrick
11 years 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' );
}

#35 in reply to: ↑ 34 ; follow-up: @rmccue
11 years 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.

#36 @SergeyBiryukov
11 years ago

In 25372:

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

props siobhyb.
see #25229.

#37 in reply to: ↑ 35 @bftrick
11 years 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! :)

#38 @DrewAPicture
11 years 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 11 years ago by DrewAPicture (previous) (diff)

#39 @helen
11 years ago

In 25410:

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

#40 @helen
11 years ago

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

#41 @helen
11 years ago

In 25411:

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

#42 @helen
11 years ago

In 25412:

No space between and duplicate_hook. see #25229

@NikV
11 years ago

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

@bftrick
11 years ago

adding 5 doc blocks to user-new.php

@Frank Klein
11 years ago

Updated patch containing only inline documentation changes.

#43 @DrewAPicture
11 years ago

@NikV: ms-delete-site.php1.diff is almost there. Looks like you missed removing the second closing parenthesis in the last part of the email content string: ###SITE_NAME###" ) ); should be ###SITE_NAME###" );.

Once that's done, your patch will be ready to go in :)

Version 0, edited 11 years ago by DrewAPicture (next)

#44 @DrewAPicture
11 years 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 :)

#45 @DrewAPicture
11 years 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.

#46 @SergeyBiryukov
11 years ago

In 25423:

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

props natejacobs.
see #25229.

#47 @SergeyBiryukov
11 years ago

In 25424:

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

props NikV.
see #25229.

@Frank Klein
11 years ago

Updated patch according to comment:44

#48 @mordauk
11 years 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

#49 follow-up: @DrewAPicture
11 years 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.

#50 @DrewAPicture
11 years ago

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

@mordauk
11 years ago

wp-admin/includes/comment.php

@mordauk
11 years ago

wp-admin/export.php

#51 in reply to: ↑ 49 @mordauk
11 years 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!

#52 @SergeyBiryukov
11 years ago

In 25429:

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

props Frank Klein.
see #25229.

#53 follow-ups: @DrewAPicture
11 years 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 11 years ago by DrewAPicture (previous) (diff)

#54 in reply to: ↑ 53 ; follow-up: @bftrick
11 years 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?

#55 in reply to: ↑ 54 @SergeyBiryukov
11 years 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.

@mordauk
11 years ago

wp-admin/includes/comment.php

@mordauk
11 years ago

wp-admin/export.php

#56 in reply to: ↑ 53 @mordauk
11 years 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 :)

#57 @SergeyBiryukov
11 years ago

In 25434:

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

props mordauk.
see #25229.

#58 follow-up: @SergeyBiryukov
11 years ago

In 25435:

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

props mordauk.
see #25229.

@bftrick
11 years ago

second pass as user-new.php

#59 in reply to: ↑ 58 @bftrick
11 years ago

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

#60 follow-up: @DrewAPicture
11 years 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!

@dustyf
11 years ago

First pass at wp-includes/canonical.php

@dustyf
11 years ago

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

@dustyf
11 years ago

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

@dustyf
11 years ago

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

@dustyf
11 years ago

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

@dustyf
11 years ago

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

@dustyf
11 years ago

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

@dustyf
11 years ago

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

@naomicbush
11 years ago

wp-admin/plugin-install.php

@naomicbush
11 years ago

wp-admin/includes/plugin-install.php

@netweb
11 years ago

wp-links-opml.php 1st pass

#61 @netweb
11 years ago

Taken a shot at wp-links-opml.php

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

#62 @netweb
11 years ago

  • Cc netweb added

#63 @Frank Klein
11 years ago

  • Cc contact@… added

@NikV
11 years ago

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

@mordauk
11 years ago

wp-includes/load.php

@mordauk
11 years ago

wp-includes/load.php

#64 @mordauk
11 years ago

Oops, missed the .diff extension.

#65 follow-up: @DrewAPicture
11 years 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.

@mordauk
11 years ago

wp-includes/load.php

#66 in reply to: ↑ 65 @mordauk
11 years 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.

#67 @DrewAPicture
11 years ago

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

#68 @SergeyBiryukov
11 years ago

In 25455:

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

props mordauk.
see #25229.

@bftrick
11 years ago

third pass of user-new.php

#69 in reply to: ↑ 60 ; follow-up: @bftrick
11 years 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. :)

#70 in reply to: ↑ 69 @DrewAPicture
11 years 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 11 years ago by DrewAPicture (previous) (diff)

@DrewAPicture
11 years ago

Ready for commit

#71 @DrewAPicture
11 years ago

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

#72 @SergeyBiryukov
11 years 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.

#73 @SergeyBiryukov
11 years ago

In 25470:

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

props bftrick.
see #25229.

#74 follow-ups: @DrewAPicture
11 years 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

#75 follow-up: @DrewAPicture
11 years 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:

#76 follow-up: @ericlewis
11 years 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!

@dllh
11 years ago

wp-admin/includes/plugin.php

#77 in reply to: ↑ 75 @dustyf
11 years 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.

@dllh
11 years 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.

#78 in reply to: ↑ 76 ; follow-up: @NikV
11 years 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 11 years ago by NikV (previous) (diff)

@betzster
11 years ago

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

#79 @westi
11 years ago

In 25474:

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

See #25229 props dllh

#80 @westi
11 years ago

In 25475:

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

See #25229 props betzster

#81 in reply to: ↑ 74 @netweb
11 years 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

#82 in reply to: ↑ 78 @NikV
11 years 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.

#83 follow-up: @SergeyBiryukov
11 years 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.

#84 @SergeyBiryukov
11 years ago

In 25477:

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

#85 @SergeyBiryukov
11 years ago

In 25478:

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

#86 @SergeyBiryukov
11 years ago

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

#87 in reply to: ↑ 83 @dllh
11 years 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.hoereth
11 years ago

wp-admin/plugin-install.php

#89 @rzen
11 years 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.

@dustyf
11 years ago

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

#90 follow-up: @rzen
11 years 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.

#91 in reply to: ↑ 90 @DrewAPicture
11 years 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.

@DrewAPicture
11 years ago

Minus the extra whitespacing

#92 @DrewAPicture
11 years ago

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

#93 @SergeyBiryukov
11 years ago

In 25479:

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

props dustyf.
see #25229.

@DrewAPicture
11 years ago

Fix @since versions

#94 @DrewAPicture
11 years ago

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

@enej
11 years ago

Second pass at ms.php

@DrewAPicture
11 years ago

Action fixes after [25474] and [25477]

#95 follow-up: @DrewAPicture
11 years 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].

@dustyf
11 years ago

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

#96 @SergeyBiryukov
11 years ago

In 25480:

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

props netweb.
see #25229.

@DrewAPicture
11 years ago

wp-admin/includes/ms.php

#97 @DrewAPicture
11 years ago

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

#98 in reply to: ↑ 95 ; follow-up: @SergeyBiryukov
11 years 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?

#99 in reply to: ↑ 98 ; follow-up: @DrewAPicture
11 years 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.

#100 @rzen
11 years 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.

#101 @SergeyBiryukov
11 years ago

In 25481:

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

props enej, DrewAPicture.
see #25229.

#102 follow-up: @SergeyBiryukov
11 years ago

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

#103 in reply to: ↑ 99 ; follow-up: @SergeyBiryukov
11 years ago

Replying to DrewAPicture:

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

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

#104 in reply to: ↑ 103 @DrewAPicture
11 years 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.

#105 @SergeyBiryukov
11 years ago

In 25482:

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

props DrewAPicture.
see #25229.

#106 @rzen
11 years 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 11 years ago by DrewAPicture (previous) (diff)

@jamescollins
11 years ago

Minor fixes/changes to [25481]

#107 in reply to: ↑ 102 @jamescollins
11 years 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.

#108 @SergeyBiryukov
11 years ago

In 25486:

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

props jamescollins.
see #25229.

#109 @SergeyBiryukov
11 years ago

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

#110 @SergeyBiryukov
11 years ago

In 25497:

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

@naomicbush
11 years ago

wp-admin/includes/plugin-install.php

@naomicbush
11 years ago

wp-admin/plugin-install.php

#111 in reply to: ↑ 74 @naomicbush
11 years 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.

@NikV
11 years ago

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

#112 follow-up: @onalioa
11 years ago

Last edited 11 years ago by DrewAPicture (previous) (diff)

#113 in reply to: ↑ 112 @DrewAPicture
11 years ago

Replying to onalioa:

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

#114 @DrewAPicture
11 years ago

In 25511:

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

Props naomicbush.
See #25229.

#115 @DrewAPicture
11 years ago

In 25512:

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

Props naomicbush.
See #25229.

#116 @DrewAPicture
11 years ago

In 25513:

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

Props NikV.
See #25229.

#117 @DrewAPicture
11 years ago

In 25529:

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

Props dustyf.
See #25229.

#118 @DrewAPicture
11 years ago

In 25530:

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

Props dustyf for the initial patch.
See #25229.

#119 @DrewAPicture
11 years ago

In 25531:

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

Props dustyf.
See #25229.

#120 @DrewAPicture
11 years ago

In 25532:

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

Props dustyf.
See #25229.

#121 @DrewAPicture
11 years ago

In 25534:

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

Props a.hoereth.
See #25229.

#122 @DrewAPicture
11 years ago

In 25535:

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

Props dustyf for the initial patch.
See #25229.

#123 @DrewAPicture
11 years 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 11 years ago by DrewAPicture (previous) (diff)

#124 follow-up: @DrewAPicture
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

#125 @DrewAPicture
11 years ago

In 25537:

Inline documentation for hooks in wp-activate.php.

Props nullvariable for the initial patch.
See #25229.

#126 @DrewAPicture
11 years 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.

#127 @DrewAPicture
11 years ago

In 25539:

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

See #25229.

#128 in reply to: ↑ 124 ; follow-up: @nullvariable
11 years ago

Replying to DrewAPicture:

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

eh my bad! Just saw that.

#129 in reply to: ↑ 128 @DrewAPicture
11 years 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 :)

#130 @dd32
11 years 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

#131 @DrewAPicture
11 years 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].

#132 @DrewAPicture
11 years 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].

#133 @DrewAPicture
11 years 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].

#134 @DrewAPicture
11 years ago

In 27147:

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

See #26869, #25229 and [25284].

#135 @DrewAPicture
11 years ago

In 27201:

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

See #26869, #25229 and [25410].

#136 @DrewAPicture
10 years 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.