Opened 7 years ago
Closed 2 years ago
#43755 closed defect (bug) (duplicate)
CS: Fix violations for wp-includes/canonical.php
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Canonical | Keywords: | has-patch |
Focuses: | coding-standards | Cc: |
Description
Started working on this at WordCamp London contributor day.
Attachments (2)
Change History (11)
#2
@
7 years ago
- Milestone changed from Awaiting Review to 5.0
- Owner set to netweb
- Status changed from new to reviewing
#6
@
6 years ago
- Keywords needs-refresh added
- Milestone changed from 5.1 to 5.2
This is going to need some careful review. A couple of notes from my brief read through:
When moving an assignment out of a conditional block, it should be moved to before the conditional, unless there are significant performance reasons to move it inside.
eg:
if ( is_feed() && ( $id = get_query_var( 'p' ) ) ) {
// ...
}
would become:
$id = get_query_var( 'p' );
if ( $id && is_feed() ) {
// ...
}
There's no need to add an empty()
call where there wasn't one before.
Similarly, there's no need to add extra type checks. If the value returned by get_permalink()
is truth-y, you can assume it's a string.
Note: See
TracTickets for help on using
tickets.
Have made some additional changes to make sure
wp_parse_url
is used on all occasions.Also to be able to ignore the
prepare
statement sniff, have made sure all parts of the query have been prepared.I can imagine this could be a separate patch or at least be a separate commit, but it was in line of fixing the code sniff.
Have ran all unit tests and did not encounter any problems.