Make WordPress Core

Opened 8 months ago

Closed 3 months ago

#59420 closed defect (bug) (fixed)

Remove unused variables on the wp-includes folder.

Reported by: upadalavipul's profile upadalavipul Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: coding-standards Cc:

Description

I have reviewed the code and found some of the variables are defined but they do not use in the below files. Please check the below files:

Files:

  1. wp-includes/ms-settings.php
  2. wp-includes/rest-api/class-wp-rest-server.php

Attachments (10)

59420.patch (1.0 KB) - added by upadalavipul 8 months ago.
59420.2.patch (477 bytes) - added by upadalavipul 8 months ago.
59420.3.patch (480 bytes) - added by viralsampat 8 months ago.
I have checked above mentioned issue and founds new files.
59420.4.patch (367 bytes) - added by upadalavipul 8 months ago.
59420.5.patch (504 bytes) - added by upadalavipul 8 months ago.
59420.6.patch (620 bytes) - added by upadalavipul 8 months ago.
59420.7.patch (2.0 KB) - added by viralsampat 8 months ago.
I have checked above mentioned issue and I have added its patch.
59420.8.patch (1.1 KB) - added by viralsampat 7 months ago.
I have checked above mentioned issue and founds few files. Here, I have added its patch.
59420.9.patch (499 bytes) - added by viralsampat 7 months ago.
I have checked above mentioned issue and founds few files. Here, I have added its patch.
59420.10.patch (1.4 KB) - added by viralsampat 7 months ago.
I have checked above mentioned issue and founds few files. Here, I have added its patch.

Download all attachments as: .zip

Change History (19)

@upadalavipul
8 months ago

#1 @mukesh27
8 months ago

The changes for wp-includes/ms-settings.php added 11 year back with the below message. I'm struggling to get the ticket

  • Define $_wp_switched_stack and $_wp_switched. This way switch_to_blog() and restore_current_blog() can rely on it being set.

$_wp_switched_stack is set as global so there may be failing something if we remove it?

The null response variable added in [34928].

Last edited 8 months ago by SergeyBiryukov (previous) (diff)

#2 @SergeyBiryukov
8 months ago

  • Milestone changed from Awaiting Review to 6.4

Hi there, thanks for the patch!

The $_wp_switched_stack and $switched globals are initialized in wp-includes/ms-settings.php for later use in switch_to_blog() and restore_current_blog() functions, see [21485]:

wp-includes/ms-settings.php:
Define $_wp_switched_stack and $_wp_switched. This way switch_to_blog() and restore_current_blog() can rely on it being set.

So they cannot be removed.

The $response variable in WP_REST_Server::match_request_to_handler(), on the other hand, indeed appears to be unused.

#3 @SergeyBiryukov
8 months ago

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

In 56645:

REST API: Remove unused variable in WP_REST_Server::match_request_to_handler().

Previously initialized in WP_REST_Server::dispatch(), the $response variable became unused when the logic was split into two new methods, ::match_request_to_handler() and ::respond_to_request().

Follow-up to [34928], [48947].

Props upadalavipul, mukesh27.
Fixes #59420.

@viralsampat
8 months ago

I have checked above mentioned issue and founds new files.

@viralsampat
8 months ago

I have checked above mentioned issue and I have added its patch.

#4 @SergeyBiryukov
8 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to review the latest patches.

This ticket was mentioned in Slack in #core by oglekler. View the logs.


7 months ago

#6 follow-up: @nicolefurlan
7 months ago

This ticket was discussed in today's bug scrub.

We'll leave this open for now and revisit closing it tomorrow if @SergeyBiryukov doesn't get time to commit these patches. In that case, a new ticket on 6.5 can be opened for the ongoing work.

Props @hellofromTonya :)

#7 @hellofromTonya
7 months ago

  • Keywords has-patch added
  • Milestone changed from 6.4 to 6.5

RC1 is about to start. Moving the remaining work to 6.5 cycle.

@viralsampat
7 months ago

I have checked above mentioned issue and founds few files. Here, I have added its patch.

@viralsampat
7 months ago

I have checked above mentioned issue and founds few files. Here, I have added its patch.

@viralsampat
7 months ago

I have checked above mentioned issue and founds few files. Here, I have added its patch.

This ticket was mentioned in Slack in #core by rajinsharwar. View the logs.


4 months ago

#9 in reply to: ↑ 6 @sabernhardt
3 months ago

  • Milestone changed from 6.5 to 6.4
  • Resolution set to fixed
  • Status changed from reopened to closed

a new ticket on 6.5 can be opened for the ongoing work.

#59297 already has plenty of overlap with this ticket, so that can be used to consider whether any additional variables should be removed.

The change from 59420.patch and 59420.2.patch is done in [56645].

59420.3.patch removes the $shortcode_tags global, but that is used in several functions, including run_shortcode() and wptexturize().

59420.4.patch proposes removing $blog_id from ms_load_current_site_and_network(), which is part of 59297.5.patch.

59420.7.patch proposes changes in multiple unrelated files:

  • To suggest removing $AudioChunkStreamNum from the getID3 RIFF module, please report that on the external library's GitHub repository.
  • The $unapproved_ids and $unapproved_emails were already removed from WP_Comment_Query::get_comment_ids() by [55559].
  • The $parent_post_type variable should not be removed from the WP_REST_Revisions_Controller class because the __construct() method uses it.

59420.9.patch proposes removing $hidden_inputs from the_block_editor_meta_box_post_form_hidden_fields(), which is part of 59297.patch.

I'll add notes about patches 5, 6, 8 and 10 on #59297.

Note: See TracTickets for help on using tickets.