Opened 11 years ago
Closed 11 years ago
#25531 closed defect (bug) (fixed)
Hook Docs: wp-includes/capabilities.php
Reported by: | ptahdunbar | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 3.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Inline Docs | Keywords: | has-patch |
Focuses: | Cc: |
Description
Attached patch adds phpdocs for all hooks in wp-includes/capabilities.php. Please review to confirm wording/correctness :)
Attachments (2)
Change History (9)
#2
@
11 years ago
- Keywords 2nd-opinion added
I'd like to see what others have to say about my recommendations for set_user_role
, user_has_cap
and auth_post_meta_{$meta_key}
in regards to naming the params different from the actual variables being used in the filter call.
My feeling is that it makes the docs more human-readable when separated from the context of the code, with the obvious trade-off that the pairing becomes awkward when viewed in code (and possibly within an IDE). This may be a good topic to discuss during a weekly Inline Docs meeting.
#3
@
11 years ago
The result from today's inline-docs meeting in #wordpress-sfd is that yes, we do want to use alternate variable names whenever a param is not cast to a variable or whenever its a class property of some kind. It is a little strange to see it side-by-side with the code, but its of much greater benefit when read outside the code.
#5
@
11 years ago
- Keywords has-patch added; needs-patch removed
25531.1.diff incorporates changes from patch review in comment:1, and corrected @since values.
Ptah, solid work!
Overall
role_has_cap
@param
variable names (e.g. $this->capabilities becomes $capabilities)set_user_role
$this->ID
param to$user_id
.user_has_cap
$this->allcaps
param to$allcaps
$this
param to$user
auth_post_meta_{$meta_key}
@param
should be named something, I suggest$allowed
.$post->ID
param can be renamed to$post_id
.