Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#37447 closed defect (bug) (fixed)

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

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

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 9 years ago.
37447.2.diff (13.4 KB) - added by flixos90 9 years ago.

Download all attachments as: .zip

Change History (13)

@flixos90
9 years ago

#1 @flixos90
9 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.


9 years ago

#3 follow-up: @SergeyBiryukov
9 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
9 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
9 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.


9 years ago

#7 @flixos90
9 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.7

#8 follow-up: @jeremyfelt
9 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
9 years ago

#9 in reply to: ↑ 8 @flixos90
9 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
8 years ago

This looks OK to me.

#11 @jeremyfelt
8 years ago

  • Owner set to jeremyfelt
  • Resolution set to fixed
  • Status changed from new to 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.