WordPress.org

Make WordPress Core

Opened 9 months ago

Last modified 3 months ago

#45059 reviewing enhancement

Changed meaningless variable name "$r"

Reported by: freewebmentor Owned by: SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: minor Version: 4.9.8
Component: General Keywords: good-first-bug has-patch
Focuses: coding-standards Cc:

Description

Changed meaningless variable name "$r" from wp-include/category-template.php file and also added the documentation for same.

Attachments (3)

45059.patch (8.9 KB) - added by freewebmentor 9 months ago.
45059.2.patch (90.5 KB) - added by freewebmentor 8 months ago.
45059.3.patch (92.3 KB) - added by freewebmentor 5 months ago.
updated meta-boxes.php and run composer run format

Download all attachments as: .zip

Change History (15)

#1 @mukesh27
9 months ago

  • Focuses coding-standards added; template removed
  • Keywords dev-feedback needs-refresh added
  • Severity changed from normal to minor
  • Summary changed from Changed meaningless variable name "$r" from wp-include/category-template.php file to Changed meaningless variable name "$r"

Hi @freewebmentor, Welcome to WordPress Trac! Thank you for your ticket.

We have to change all meaningless variables in core as per coding standards.

Also there are many files which used $r as array so if we have to change $r variable then we have to change that variable from all files.

For your information WordPress core wp-includes folder 33 files and wp-admin folder 12 files contain $r variables.

#2 @SergeyBiryukov
8 months ago

  • Keywords needs-patch good-first-bug added; has-patch dev-feedback needs-refresh removed
  • Milestone changed from Awaiting Review to Future Release

$r is used so that the original $args array is still available to be passed to various hooks.

Newer code uses $parsed_args for that, see [41733] for example.

So let's change $r to $parsed_args across the older code as well for consistency.

#3 follow-up: @freewebmentor
8 months ago

@SergeyBiryukov , Thanks for quick review feedback, So I need to update the $r from all the files like wp-includes folder 33 files and wp-admin folder 12 files contain $r variables?

#4 in reply to: ↑ 3 @SergeyBiryukov
8 months ago

Replying to freewebmentor:

So I need to update the $r from all the files like wp-includes folder 33 files and wp-admin folder 12 files contain $r variables?

At a glance, $r is not always used with wp_parse_args() in those files, sometimes it's a shorthand for $result or $response.

In this ticket, I'd suggest focusing on the instances where $r is used with wp_parse_args(). Once that's done, we can create new tickets for other instances.

#5 @freewebmentor
8 months ago

@SergeyBiryukov As requested, Please find the updated patch for this changes.

#6 @SergeyBiryukov
8 months ago

  • Milestone changed from Future Release to 5.1
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

Thanks for the patch!

#7 @SergeyBiryukov
8 months ago

  • Keywords has-patch added; needs-patch removed

#8 follow-up: @pento
5 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.1 to 5.2

In 45059.2.patch, I noticed in meta-boxes.php that some of the old references to $r haven't been updated to $parse_args.

Also, this patch has some code formatting issues. It'd be good to run composer run format on it.

#9 in reply to: ↑ 8 @freewebmentor
5 months ago

Thanks @pento,

I will make the suggested changes and will add the updated Patch.

Replying to pento:

In 45059.2.patch, I noticed in meta-boxes.php that some of the old references to $r haven't been updated to $parse_args.

Also, this patch has some code formatting issues. It'd be good to run composer run format on it.

#10 @freewebmentor
5 months ago

Hi @pento

I have updated the meta-boxes.php that some of the old references to $r haven't been updated to $parse_args and also run the composer run format. Please find the updated patch.

@freewebmentor
5 months ago

updated meta-boxes.php and run composer run format

#11 @freewebmentor
5 months ago

  • Keywords needs-refresh removed

#12 @SergeyBiryukov
3 months ago

  • Milestone changed from 5.2 to 5.3

Missed the 5.2 Beta 1 deadline, moving to 5.3.

Note: See TracTickets for help on using tickets.