Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#56448 closed enhancement (fixed)

Use meaningful variable name for $c in WP_Users_List_Table class

Reported by: burhandodhy's profile burhandodhy Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch
Focuses: Cc:

Description

Currently, the functions get_columns and get_sortable_columns in the WP_Users_List_Table class have a variable $c for the columns. We need to update the names so they will have more meaning.

Change History (14)

This ticket was mentioned in PR #3144 on WordPress/wordpress-develop by burhandodhy.


2 years ago
#1

  • Keywords has-patch added

#2 follow-up: @costdev
2 years ago

  • Component changed from General to Users
  • Milestone changed from Awaiting Review to 6.1

Thanks for opening the ticket @burhandodhy !

This makes sense. I'd also suggest that we change $r to $row in ::single_row().

This ticket was mentioned in PR #3146 on WordPress/wordpress-develop by mukeshpanchal27.


2 years ago
#3

Trac ticket:

#4 in reply to: ↑ 2 @mukesh27
2 years ago

Replying to costdev:

This makes sense. I'd also suggest that we change $r to $row in ::single_row().

https://github.com/WordPress/wordpress-develop/pull/3146 address $r to $row changes. Can you please review the PR?

#5 follow-up: @Presskopp
2 years ago

Is this part of a policy?

Because there are much more places using a single character as a variable name in core...

Examples:
$c (blog count) in admin.php
$x (ajax response) in ajax-actions.php

#6 in reply to: ↑ 5 @SergeyBiryukov
2 years ago

Replying to Presskopp:

Is this part of a policy?

I think it falls under this recommendation in Naming Conventions:

Don’t abbreviate variable names unnecessarily; let the code be unambiguous and self-documenting.

There have been some efforts to use more meaningful names for some of the older variables like this as part of general coding standards fixes in tickets like #55647, so I would welcome any further fixes, preferably in separate PRs per file, for easier review.

#8 @SergeyBiryukov
2 years ago

In 54070:

Coding Standards: Use more meaningful variable names in WP_Users_List_Table.

This renames some variables for clarity, per the Naming Conventions:

Don’t abbreviate variable names unnecessarily; let the code be unambiguous and self-documenting.

  • $c is renamed to $columns in ::get_columns() and ::get_sortable_columns().
  • $r is renamed to $row in ::single_row().

Follow-up to [3677], [5542], [6852], [8936], [15491], [16573].

Props burhandodhy, costdev, mukesh27, Presskopp.
See #56448, #55647.

SergeyBiryukov commented on PR #3144:


2 years ago
#9

Thanks for the PR! Merged in r54070.

SergeyBiryukov commented on PR #3146:


2 years ago
#10

Thanks for the PR! Merged in r54070.

#11 @SergeyBiryukov
2 years ago

In 54071:

Coding Standards: Use more meaningful variable names for output in the admin.

This renames some variables for clarity, per the Naming Conventions:

Don’t abbreviate variable names unnecessarily; let the code be unambiguous and self-documenting.

  • $out is renamed to $output in various list table methods and admin functions.
  • $sep is renamed to $separator in various list table methods and admin functions.

This affects:

  • WP_Comments_List_Table::handle_row_actions()
  • WP_List_Table::row_actions()
  • WP_Media_List_Table::column_default()
  • WP_MS_Sites_List_Table::site_states()
  • WP_MS_Users_List_Table::column_blogs()
  • WP_Terms_List_Table::column_name()
  • _wp_dashboard_recent_comments_row()
  • image_align_input_fields()
  • image_size_input_fields()
  • wp_doc_link_parse()
  • _post_states()
  • _media_states()

Follow-up to [8653], [8692], [8864], [8910], [8911], [8916], [9103], [9153], [10607], [15491], [17793], [32644], [54070].

Props mukesh27, costdev.
See #56448, #55647.

SergeyBiryukov commented on PR #3180:


2 years ago
#12

Thanks for the PR! Merged in r54071.

#13 @SergeyBiryukov
2 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#14 @SergeyBiryukov
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

Going to close this as fixed, since the original PRs are committed.

Let's open new tickets for any follow-up fixes, or just use #55647. Thanks!

Note: See TracTickets for help on using tickets.