Opened 7 years ago
Closed 7 years ago
#43566 closed defect (bug) (invalid)
CS: Fix elseif in WordPress core files
Reported by: | mukesh27 | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-patch |
Focuses: | docs, coding-standards | Cc: |
Description
CS: Fix elseif in WordPress core files
Many apologies if this is a duplicate. I have searched but did not find it yet posted.
According to PHP Coding Standards( https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#use-elseif-not-else-if ) handbook else if is not compatible with the colon syntax for if|elseif blocks. For this reason, use elseif for conditionals. in many place core developer use else if so we have to change else if to elseif or remove Use elseif, not else if block from PHP Coding Standards document.
Attachments (1)
Change History (8)
#4
@
7 years ago
@mukesh27 sorry if I misread the ticket, but in the changeset link I posted previously we can see that one of the changes made there in wp-includes/admin-bar.php
is:
384 } elseif ( current_user_can( 'read' ) ) { 385 // We're on the front end, link to the Dashboard. 386 $wp_admin_bar->add_menu( 387 array(
There's also elseif
in the latest trunk as well:
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/admin-bar.php#L384
Is this the same part as in your patch?
I tried to look for other cases of else if
in the core, but only found it in javascript code.
Did you maybe find other cases in core PHP files?
#5
@
7 years ago
i am checking in WordPress 4.9.4 and found in below files and some other in 4.9.4
/wp-includes/atomlib.php
/wp-includes/class-wp-customize-control.php
/wp-includes/class-wp-customize-manager.php
/wp-includesclass-wp-customize-widgets.php
/wp-includesclass-wp-hook.php
/wp-includes/customize/class-wp-customize-partial.php
And in trunk found below file also some other files is there.
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/atomlib.php#L249
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/IXR/class-IXR-server.php#L122
#6
@
7 years ago
Thanks for the list.
I think it's best to have the latest trunk.
Changes are usually not made to 3rd party libraries.
Here is a list of 3rd party libraries in phpcs.xml.dist
:
https://core.trac.wordpress.org/browser/trunk/phpcs.xml.dist#L16
including:
26 <exclude-pattern>/src/wp-includes/atomlib\.php</exclude-pattern> 42 <exclude-pattern>/src/wp-includes/IXR/*</exclude-pattern>}}}
#7
@
7 years ago
- Milestone Awaiting Review deleted
- Resolution set to invalid
- Status changed from new to closed
Thanks @mukesh27. As @birgire said, patches must be generated from a checkout of the development repository, which is the trunk version of WordPress with all the latest changes. Unfortunately, creating patches from a released version is not recommended, as many things have changed in the codebase in the meantime.
Specifically, a recent change that will be released with WordPress 5.0 fixed many coding standards in the codebase, see
https://core.trac.wordpress.org/changeset/42343
and for the admin bar:
https://core.trac.wordpress.org/changeset/42343/trunk/src/wp-includes/admin-bar.php
Worth also noting files related to external libraries are not required to meet the coding standards.
You can find more details about the recommended practices to contribute to WordPress in the developers handbook:
https://make.wordpress.org/core/handbook/contribute/
How to install manually a copy of trunk from the development repository:
https://make.wordpress.org/core/handbook/tutorials/installing-wordpress-locally/from-svn/#1-check-out-wordpress-trunk
Or, just use VVV:
https://make.wordpress.org/core/handbook/tutorials/installing-a-local-server/installing-vvv/
Going to close this ticket since your proposed change is already in core :) Thanks again.
Thanks for the report and the patch @mukesh27
Looking at this part of changeset [42343]:
https://core.trac.wordpress.org/changeset/42343/trunk/src/wp-includes/admin-bar.php
it seems like it was fixed there or is this a different instance?