Opened 10 years ago
Closed 10 years ago
#31248 closed defect (bug) (fixed)
PHPDoc improvements for wp-admin/includes/template.php
Reported by: | ipm-frommen | Owned by: | DrewAPicture |
---|---|---|---|
Milestone: | 4.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-patch |
Focuses: | docs | Cc: |
Description
Originally, I just wanted to replace the missing short description of the get_inline_data
function with the comment directly placed above the PHPDoc block. But then I fixed several other things that either my IDE complained about or that I just saw while plowing through the code.
Applying the attached patch does the following:
- add short description to
get_inline_data
function; - replace default value
'1'
ofint
variable$position
inwp_comment_reply
function with1
; - replace empty
return;
statements for_list_meta_row
function with@return string
PHPDoc comment withreturn '';
; - replace default value
false
ofstring
variable$selected
inwp_dropdown_roles
function with''
; - replace loose comparison with strict string comparison in
wp_dropdown_roles
function; - remove unused (last!) argument
$limit_styles
iniframe_header
function (as well as the according PHPDoc comment); - remove unused globalization of
$current_user
variable iniframe_header
function; - add missing
@return string
PHPDoc comment forget_submit_button
function; - replace default values
null
ofstring
variable$text
andarray
variable$other_attribues
inget_submit_button
function with appropirate values (i.e.,''
andarray()
); - add missing
@param
PHPDoc comments forenqueue_scripts
anddismiss_pointers_for_new_users
functions.
Attachments (4)
Change History (17)
#2
@
10 years ago
#3
@
10 years ago
The new patch does almost exactly what the first one does. The only difference is that it marks the unused parameter in the iframe_header
function unused/deprecated and does not remove it.
Besides that, is there anything wrong with this ticket?
#4
follow-ups:
↓ 5
↓ 7
↓ 9
@
10 years ago
Just a cursory glance at the patch reveals that there are code changes mixed in with doc changes. Pretty much all the cases where you've changed default values for parameters or expected returns within functions would require also having accompanying unit tests.
So I would suggest splitting the docs changes from the code changes into separate patches and probably separate tickets. we're trying to move away from these types of general-purpose optimizations unless it's a blessed task.
#5
in reply to:
↑ 4
@
10 years ago
Replying to DrewAPicture:
there are code changes mixed in with doc changes. Pretty much all the cases where you've changed default values for parameters or expected returns within functions would require also having accompanying unit tests.
Well, what these changes are all about is that the overall behavior of the code/functions does not change at all, which means that the existing unit tests are all working absolutely fine.
I would suggest splitting the docs changes from the code changes into separate patches and probably separate tickets.
Okay, that's why I asked if something was wrong; I silently assumed something like this. I will update ticket and patch, and open a new ticket (or maybe two) for parts of the changes.
I just wasn't sure what was best: one ticket handling a bunch of changes in a single file, or several tickets for different changes in the same file. Anyway, now I know what you prefer. Thanks. :)
#6
@
10 years ago
- Focuses template removed
- Summary changed from Optimizations for wp-admin/includes/template.php to PHPDoc improvements for wp-admin/includes/template.php
#7
in reply to:
↑ 4
@
10 years ago
Replying to DrewAPicture:
I would suggest splitting the docs changes from the code changes...
I just did that.
#9
in reply to:
↑ 4
@
10 years ago
Replying to DrewAPicture:
I would suggest splitting the docs changes from the code changes...
Is there anything unclear with this ticket?
I edited the title, but since I cannot edit the ticket description, it might not be clear what the ticket now is about. The last patch only contains PHPDoc changes. Any other aspect has been taken care of in different tickets already.
#10
@
10 years ago
- Milestone changed from Awaiting Review to 4.2
- Owner set to DrewAPicture
- Status changed from new to reviewing
I also created a Pull Request on GitHub.