Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#25468 closed defect (bug) (fixed)

Hook Docs: wp-includes/bookmark.php

Reported by: shinichin's profile ShinichiN Owned by: rzen's profile rzen
Milestone: 3.8 Priority: normal
Severity: normal Version:
Component: Inline Docs Keywords: has-patch commit
Focuses: Cc:

Description

Inserting inline documents for all the hooks in wp-includes/bookmark.php

Attachments (5)

25468.diff (3.5 KB) - added by ShinichiN 10 years ago.
25468-2.diff (2.9 KB) - added by ShinichiN 10 years ago.
Fixes the #25468 patch according to the review.
25468-3.diff (3.2 KB) - added by ShinichiN 10 years ago.
Added white spaces before/after the arguments of filters.
25468.4.diff (2.3 KB) - added by kpdesign 10 years ago.
Second pass on 25468-3.diff
25468.2.diff (2.5 KB) - added by DrewAPicture 10 years ago.
fixed duplicate tags

Download all attachments as: .zip

Change History (12)

@ShinichiN
10 years ago

#1 @DrewAPicture
10 years ago

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

#2 @DrewAPicture
10 years ago

  • Keywords needs-patch added; has-patch removed
  • Type changed from enhancement to defect (bug)

Hi, thanks for the patch. Some notes on 25468.diff:

  • Avoid making changes to functions and functional docs, especially whitespacing. Focus on the hooks :)

get_bookmarks filter:

  • Since this is a "named" filter hook for a function, and used more than once, the main doc block should cover the various contexts it's evaluated in. So for the short description, maybe "Filter the returned list of bookmarks."
  • Use a long description (but not too long!) to describe the other contexts. For instance, the first instance of the filter returns the cached bookmarks list. The second returns a cached bookmarks list if the link category is passed but doesn't exist. The third returns the full cached results.
  • $r is an array of arguments passed to get_bookmarks() so just back-reference that, like this:
    * @param array $r         An array of bookmark query arguments. @see get_bookmarks() 
    

edit_$field, pre_$field and $field should just be //duplicate_hook

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

@ShinichiN
10 years ago

Fixes the #25468 patch according to the review.

#3 follow-up: @ShinichiN
10 years ago

  • Keywords has-patch added; needs-patch removed

Thank you for the review.

Is it ok to make change as below to the hooks?

before
apply_filters('get_bookmarks', $results, $r);

after
apply_filters( 'get_bookmarks', $results, $r );
just inserting spaces before and after the arguments.

#4 in reply to: ↑ 3 @DrewAPicture
10 years ago

Replying to ShinichiN:

Thank you for the review.

Is it ok to make change as below to the hooks?

before
apply_filters('get_bookmarks', $results, $r);

after
apply_filters( 'get_bookmarks', $results, $r );
just inserting spaces before and after the arguments.

Yep, sure is :)

@ShinichiN
10 years ago

Added white spaces before/after the arguments of filters.

@kpdesign
10 years ago

Second pass on 25468-3.diff

@DrewAPicture
10 years ago

fixed duplicate tags

#5 @DrewAPicture
10 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 3.8

#6 @DrewAPicture
10 years ago

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

In 25934:

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

Props ShinichiN, kpdesign.
Fixes #25468.

#7 @dd32
10 years ago

In 26046:

Add some missing braces to get_bookmarks() which was causing an early return. Introduced in [25934]. See #25468. Fixes #25874

Note: See TracTickets for help on using tickets.