Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#37447 closed defect (bug) (fixed)

Remove redundant `is_multisite()` checks in network admin templates

Reported by: flixos90 Owned by: jeremyfelt
Priority: normal Milestone: 4.7
Component: Administration Version: 3.1
Severity: normal Keywords: has-patch
Cc: Focuses: multisite

Description

Most of the templates for the network admin have a check for is_multisite() in the beginning of the file. However there's already such a check happening in the admin.php file for the network admin, making these additional checks redundant. Let's remove them.

Attachments (2)

37447.diff​ (12.7 KB ) - added by flixos90 10 years ago.
37447.2.diff​ (13.4 KB ) - added by flixos90 10 years ago.

Download all attachments as: .zip

Change History (13)

@flixos90
10 years ago

#1 @flixos90
10 years ago

  • Keywords has-patch added; needs-patch removed

37447.diff​ removes all the unnecessary is_multisite() checks.

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


10 years ago

#3 follow-up: @SergeyBiryukov
10 years ago

Originally introduced in [12609], expanded in [16272]. network/admin.php was added in [15481].

Looking at [16272], I guess the checks are there just in case the one in network/admin.php gets removed at some point (it might be an oversight as well, but seems unlikely).

#4 in reply to: ↑ 3 ; follow-up: @flixos90
10 years ago

Replying to SergeyBiryukov:

Looking at [16272], I guess the checks are there just in case the one in network/admin.php gets removed at some pount (it might be an oversight as well, but seems unlikely).

Why would that file be removed? Solely for that purpose, (so that we don't need duplicate code) I think it would make sense to keep it.

#5 in reply to: ↑ 4 @swissspidy
10 years ago

Replying to flixos90:

Replying to SergeyBiryukov:

Looking at [16272], I guess the checks are there just in case the one in network/admin.php gets removed at some pount (it might be an oversight as well, but seems unlikely).

Why would that file be removed? Solely for that purpose, (so that we don't need duplicate code) I think it would make sense to keep it.

Not the file could be removed, but the checks in it.

This ticket was mentioned in ​Slack in #core-multisite by flixos90. ​View the logs.


10 years ago

#7 @flixos90
10 years ago

  • Keywords commit added
  • Milestone Awaiting Review → 4.7

#8 follow-up: @jeremyfelt
10 years ago

  • Keywords commit removed

I think we're okay to remove these. It seems (IMO) like part of [16272] was about applying some consistency to network admin files that didn't have both the is_multisite() check and the bootstrap documentation. It doesn't really hurt to to have the 2 checks, but the opportunity to reduce code is nice. :)

Is there a way we can work in a one line comment above the check in wp-admin/network/admin.php to clarify its importance?

@flixos90
10 years ago

#9 in reply to: ↑ 8 @flixos90
10 years ago

Replying to jeremyfelt:

Is there a way we can work in a one line comment above the check in wp-admin/network/admin.php to clarify its importance?

I added such a note in 37447.2.diff​.

#10 @johnjamesjacoby
10 years ago

This looks OK to me.

#11 @jeremyfelt
10 years ago

  • Owner set to jeremyfelt
  • Resolution → fixed
  • Status new → closed

In 38657:

Multisite: Remove redundant is_multisite() checks in network admin templates.

wp-admin/network/admin.php is required by all of the individual network templates and begins with an is_multisite() check of its own. Because of this, we can remove the 26 other checks in the individual templates.

Props flixos90.
Fixes #37447.

Note: See TracTickets for help on using tickets.