#25158 closed defect (bug) (fixed)
Use get_current_site() instead of the $current_site global when possible
Reported by: | jeremyfelt | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 3.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Multisite | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
Via prod from Nacin, this does seem like a good cleanup opportunity
Patch removes almost all uses of the global $current_site
and replaces with get_current_site()
.
Notes:
is_main_site()
doesn't necessarily have access toget_current_site()
when first used and therefore requires the global$current_site
.populate_network()
sets parts of the$current_site
global when transitioning from single site to multisite and therefore needs access.get_current_site()
obviously needs access to the globalget_current_site_name()
uses a local$current_site
object that is passed to itwpmu_current_site()
is responsible for setting up the$current_site
global and therefore appears to need access. However, I'm not entirely convinced it really is required aswpmu_current_site()
is only called once by core and that is to set the global inms-settings.php
.ms-settings.php
is where$current_site
hits the global scope, so it stays.
No new failures arise in tests for both single and multisite mode. There were a couple places where I made small code formatting tweaks, though I tried to restrain. :)
Attachments (6)
Change History (24)
#2
in reply to:
↑ 1
@
11 years ago
Replying to nacin:
If a function is repeatedly calling get_current_site(), one option is to also assign get_current_site() to a local variable $current_site
Yeah, I started down that road, but then went all in. There are a few places where that would likely make sense though.
#3
follow-up:
↓ 4
@
11 years ago
25158.2.diff makes use of a local $current_site = get_current_site();
when it makes sense rather than calling get_current_site()
each time. There are a couple places where I've left multiple function calls in place as there are situations where only one of them will be called depending on logic.
Also removed an unused $wpdb
global in ms_site_check()
#4
in reply to:
↑ 3
;
follow-up:
↓ 5
@
11 years ago
Replying to jeremyfelt:
25158.2.diff makes use of a local
$current_site = get_current_site();
when it makes sense rather than callingget_current_site()
each time.
I'd suggest doing the same in all the instances. It seems more future-proof in case more instances are added later in the same function.
#5
in reply to:
↑ 4
@
11 years ago
Replying to SergeyBiryukov:
I'd suggest doing the same in all the instances. It seems more future-proof in case more instances are added later in the same function.
I think this is a judgment call; sometimes it may make sense, sometimes it is just an unnecessary variable. Really no different than when we were replacing $current_screen with get_current_screen() or $post with get_post().
#6
@
11 years ago
Indeed. I would probably keep the variable in network_site_url()
and network_home_url()
, where it's used twice in the same line. Otherwise, 25158.2.diff looks good to me.
#7
@
11 years ago
25158.3.diff addresses the changes suggested by Sergey and refreshes against current trunk.
#9
follow-up:
↓ 10
@
11 years ago
25158.4.diff keeps the variable in wpmu_delete_blog()
, where it's also used twice in the same line.
We should probably keep the global in admin-header.php
, see [18526] and #16143.
#10
in reply to:
↑ 9
@
11 years ago
Replying to SergeyBiryukov:
We should probably keep the global in
admin-header.php
, see [18526] and #16143.
On second thought, removing the global seems fine.
#11
@
11 years ago
- Owner set to SergeyBiryukov
- Status changed from new to assigned
Sergey said he was going to go through this.
#15
@
11 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
25158.5.diff reverts [26124] and checks for is_multisite()
before calling get_current_site()
. Also removes the global from is_main_network()
.
#16
@
11 years ago
Looks like we don't actually need the variable in wp-admin/admin-header.php
and can replace it with get_current_site()
there too: 25158.6.diff.
If a function is repeatedly calling get_current_site(), one option is to also assign get_current_site() to a local variable $current_site, and remove the globalization, at which point you won't need to change the function otherwise. This is how we resolved using get_post() instead of the global $post without rewriting dozens of $post references.