Make WordPress Core

Opened 8 years ago

Closed 2 years ago

Last modified 2 years ago

#37861 closed enhancement (fixed)

Renaming of class files for consistency

Reported by: afragen's profile afragen Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch
Focuses: coding-standards Cc:

Description

I'm not sure how relevant this is in general, but with #36335 shouldn't we at least follow the naming convention for the few class files left?

A quick glance shows

wp-includes/class.wp-dependencies.php -> wp-includes/class-wp-dependencies.php
wp-includes/class.wp-scripts.php -> wp-includes/class-wp-scripts.php
wp-includes/class.wp-styles.php -> wp-includes/class-wp-styles.php

I'm not certain what type of breakage may occur, unless someone is loading these independently of core.

Attachments (1)

37861.patch (3.8 KB) - added by schlessera 7 years ago.
Rename offending files and adapt all mentions

Download all attachments as: .zip

Change History (12)

#1 @swissspidy
8 years ago

  • Keywords dev-feedback added
  • Version 4.7 deleted

@schlessera
7 years ago

Rename offending files and adapt all mentions

#2 @desrosj
7 years ago

I am generally in favor of this, but I think we should at least scan the .org repository for any plugins including or requiring these files directly. If there are many, we need to properly communicate the change with enough notice.

Last edited 7 years ago by desrosj (previous) (diff)

#3 @swissspidy
7 years ago

I think we'd just copy the files using svn (to keep the history) and then require the new ones inside the old files for back compat. That's what was done in the past when classes were moved to other files.

#4 @SergeyBiryukov
2 years ago

#56374 was marked as a duplicate.

#5 @SergeyBiryukov
2 years ago

  • Focuses coding-standards added
  • Milestone changed from Awaiting Review to 6.1

Hi there, thanks for the ticket!

It's worth noting that these files are also included in the BackPress library, which may or may not be relevant as it does not seem to be actively supported.

I don't have a strong opinion here, but if there is a general consensus that these should be renamed for consistency, might as well do it. Moving to 6.1 for discussion.

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

#6 @hellofromTonya
2 years ago

  • Keywords has-patch changes-requested added; dev-feedback removed

@SergeyBiryukov I agree +1. Let's get these filenames consistent.

To avoid things breaking in plugins, I'm +1 for @swissspidy suggestion:

I think we'd just copy the files using svn (to keep the history) and then require the new ones inside the old files for back compat. That's what was done in the past when classes were moved to other files.

That extra step has precedence and can mitigate risks for users who have a plugin, theme or script that includes any of these renamed files.

#7 follow-up: @hellofromTonya
2 years ago

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

I'll work on an updated patch tomorrow for consideration. Assigning ownership to me to remind me (lol) and to help shepherd it forward in time for 6.1 feature freeze.

Last edited 2 years ago by hellofromTonya (previous) (diff)

#8 in reply to: ↑ 7 @SergeyBiryukov
2 years ago

Replying to hellofromTonya:

I'll work on an updated patch tomorrow for consideration. Assigning ownership to me to remind me (lol) and to help shepherd it forward in time for 6.1 feature freeze.

Great! Please note that svn mv commands may not be properly reflected in a patch. See comment:33:ticket:47632 for a general procedure with these renamings.

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


2 years ago

#10 @SergeyBiryukov
2 years ago

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

In 54254:

Coding Standards: Rename WordPress Dependencies API class files.

The current coding standards note that the name of the class files should be based on the class name with class- prepended, and the underscores replaced by hyphens (see the Naming Conventions section in the handbook), except for the three legacy files: class.wp-dependencies.php, class.wp-scripts.php, class.wp-styles.php.

To bring more consistency to the codebase and make it easier to implement autoloading in the future, this commit renames those three legacy files to conform to the coding standards:

  • wp-includes/class.wp-dependencies.phpwp-includes/class-wp-dependencies.php
  • wp-includes/class.wp-scripts.phpwp-includes/class-wp-scripts.php
  • wp-includes/class.wp-styles.phpwp-includes/class-wp-styles.php

Includes:

  • Loading the new files from the old ones, for anyone that may have been including the files directly.
  • Replacing references to the old filenames with the new filenames.

Follow-up to [7970], [45654], [45662], [45663], [45678], [47197], [52026], [53749].

Props afragen, schlessera, swissspidy, dingo_d, hellofromTonya, SergeyBiryukov.
Fixes #37861. See #55647.

#11 @hellofromTonya
2 years ago

  • Keywords changes-requested removed

Removing the changes-requested keyword as changes were applied to [54254]. Thank you everyone for your contributions to get this one over the finish line and into WP 6.1.

Note: See TracTickets for help on using tickets.