WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#38681 closed enhancement (maybelater)

Rename WP_User::for_blog() to WP_User::for_site()

Reported by: flixos90 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch has-unit-tests
Focuses: multisite Cc:
PR Number:

Description

Related to #38645, the name WP_User::for_blog() method follows the old naming conventions (what used to be called a blog is now called a site) and I think this one makes for a rather straightforward renaming process. I think we should rename the method to for_site() so that this new name is the preferred way of using that functionality and is highlighted in IDEs. The for_blog() should continue to work as a magic method for backward compatibility.

I think it should also be deprecated, but we could also do this change silently.

Attachments (2)

38681.diff (3.9 KB) - added by flixos90 3 years ago.
38681.2.diff (4.8 KB) - added by flixos90 3 years ago.

Download all attachments as: .zip

Change History (8)

@flixos90
3 years ago

#1 @flixos90
3 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

38681.diff renames the method and adds a handler in the __call() method for the old for_blog() method. It has not been deprecated at this point (not sure yet).

In addition I renamed all instances of $blog_id to $site_id so that the WP_User class is consistent.

#2 @johnjamesjacoby
3 years ago

I like it.

#3 @flixos90
3 years ago

  • Milestone changed from Awaiting Review to 4.8

Putting this into 4.8 since it is related to #38645, should probably happen before.

@flixos90
3 years ago

#4 @flixos90
3 years ago

  • Keywords has-unit-tests added; 2nd-opinion removed

38681.2.diff adds unit tests for using both the old and the new method name.

While this change on its own does not make sense as we shouldn't do plain refactoring, it is worth doing in regards to #38645. New code should use the new multisite naming conventions (site instead of blog), and the inconsistency in naming that we would get from introducing the new method there is worse than the change of renaming here.

Let's commit this together with #38645 once it's ready.

#5 @flixos90
3 years ago

  • Milestone 4.8 deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Per discussion at WCUS Contributor Day, we won't rename the method. Instead we will use for_blog() as name for the new method introduced in #38645 to have parity. A rename process can happen once we start implementing more network-related features (switch_to_network() for example) as many of these functions will also be changed internally a bit.

Note: See TracTickets for help on using tickets.