WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#28749 closed defect (bug) (fixed)

Unused variable $num_ratings in WP_Theme_Install_List_Table

Reported by: Corphi Owned by: ocean90
Milestone: 4.0 Priority: normal
Severity: trivial Version: 3.8
Component: Themes Keywords:
Focuses: Cc:
PR Number:

Description

WP_Theme_Install_List_Table::install_theme_info() doesn’t use its local variable $num_ratings. It seems to be superseded by wp_star_rating().

Attachments (4)

num_ratings.patch (692 bytes) - added by Corphi 5 years ago.
28749.diff (15.4 KB) - added by michalzuber 5 years ago.
svn rm file and imported filter documentation
28749.1.diff (22.3 KB) - added by michalzuber 5 years ago.
Only install_themes_upload() is used in wp-admin/theme-install.php
28749.2.diff (904 bytes) - added by michalzuber 5 years ago.
Replace inline doc

Download all attachments as: .zip

Change History (16)

@Corphi
5 years ago

#1 @SergeyBiryukov
5 years ago

  • Version changed from 3.9.1 to 3.8

Introduced in [26380].

Well, the whole file is unused since [27499], not sure if it still makes sense to patch it.

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#2 @Corphi
5 years ago

Well, if the file is unused, shouldn’t it be removed?

#3 @michalzuber
5 years ago

It's not included

wordpress-svn$ grep -r class-wp-theme-install-list-table src
src/wp-admin/includes/ajax-actions.php:2580:    /** This filter is documented in wp-admin/includes/class-
wp-theme-install-list-table.php */

Tried other variatons, maybe required with some combination with a variable.

wordpress-svn$ grep -r class-wp-theme-install src
No result
wordpress-svn$ grep -r class-wp-theme src
src/wp-settings.php:119:require( ABSPATH . WPINC . '/class-wp-theme.php' );
wordpress-svn$ grep -r theme-install-list src
No result
wordpress-svn$ grep -r install-list src
No result

The class is used

wordpress-svn$ grep -r WP_Theme_Install_List_Table src
src/wp-admin/includes/class-wp-theme-install-list-table.php:10:class WP_Theme_Install_List_Table extends WP_Themes_List_Table
src/wp-admin/includes/list-table.php:32:   'WP_Theme_Install_List_Table' => array( 'themes', 'theme-install' ),
src/wp-admin/includes/theme-install.php:158:  $wp_list_table = _get_list_table('WP_Theme_Install_List_Table');
src/wp-admin/includes/theme-install.php:173:  $wp_list_table = _get_list_table('WP_Theme_Install_List_Table');
src/wp-admin/includes/theme-install.php:199:  $wp_list_table = _get_list_table('WP_Theme_Install_List_Table');
Last edited 5 years ago by michalzuber (previous) (diff)

@michalzuber
5 years ago

svn rm file and imported filter documentation

#5 @michalzuber
5 years ago

Function in following files

wordpress-svn$ grep -r display_theme src
src/wp-admin/includes/theme-install.php:154:function display_theme( $theme ) {
src/wp-admin/includes/theme-install.php:169:function display_themes() {
src/wp-admin/includes/theme-install.php:179:// add_action('install_themes_search', 'display_themes');
src/wp-admin/includes/theme-install.php:180:// add_action('install_themes_featured', 'display_themes');
src/wp-admin/includes/theme-install.php:181:// add_action('install_themes_new', 'display_themes');
src/wp-admin/includes/theme-install.php:182:// add_action('install_themes_updated', 'display_themes');
wordpress-svn$ grep -r install_theme_information src
src/wp-admin/includes/theme-install.php:189:function install_theme_information() {
src/wp-admin/includes/theme-install.php:205:add_action('install_themes_pre_theme-information',
 'install_theme_information');

Hook in the same file only

wordpress-svn$ grep -r install_themes_pre_theme src
src/wp-admin/includes/theme-install.php:170:add_action('install_themes_pre_theme-information',
'install_theme_information');

#6 @michalzuber
5 years ago

Only install_themes_upload() is used from wp-admin/includes/theme-install.php, everything else is unused.

@michalzuber
5 years ago

Only install_themes_upload() is used in wp-admin/theme-install.php

#7 @ocean90
5 years ago

  • Keywords close added

We can't remove the file for back compat reasons. Sounds like wontfix.

@michalzuber
5 years ago

Replace inline doc

#8 @michalzuber
5 years ago

Thanks for the info ocean90 ;-)
28749.2.diff might be commited, replace with proper inline doc.

This ticket was mentioned in IRC in #wordpress-dev by michalzuber. View the logs.


5 years ago

#10 @DrewAPicture
5 years ago

  • Keywords close removed

Hi michalzuber,

The filter doc currently lives in the file it was introduced for -- the second-usage in ajax-actions is merely a duplicate. It doesn't matter if the file is currently loaded or not, if it's still available in core, it's still the correct place to document that hook.

-1 for 28749.2.diff

On track with the topic of this ticket, I can see applying num_ratings.patch as long as it doesn't create an issue elsewhere.

#11 @ocean90
5 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 29173:

Remove unused variable in WP_Theme_Install_List_Table::install_theme_info(). Unused since [26380].

props Corphi.
fixes #28749.

#12 @ocean90
5 years ago

  • Milestone changed from Awaiting Review to 4.0
Note: See TracTickets for help on using tickets.