WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#12519 closed defect (bug) (fixed)

wp_unregister_GLOBALS violates with specs

Reported by: hakre Owned by: dd32
Milestone: 3.0 Priority: lowest
Severity: trivial Version: 3.0
Component: General Keywords: has-patch tested
Focuses: Cc:

Description

The function wp_unregister_GLOBALS contains uppercase letters which breaks the specs which call for all functions containing lowercase letters only and using underscores to separate words.

Since function names are case insensitive in PHP, this can be easily fixed.

Additionally some other of the more basic Wordpress coding standards are not followed in that function which can be fixed.

Attachments (2)

12519.patch (1.2 KB) - added by hakre 10 years ago.
12519.2.patch (1.7 KB) - added by hakre 10 years ago.

Download all attachments as: .zip

Change History (14)

@hakre
10 years ago

#2 @scribu
10 years ago

  • Milestone changed from Unassigned to 3.0
  • Severity changed from normal to trivial

#3 @scribu
10 years ago

  • Priority changed from normal to lowest

#4 follow-up: @nacin
10 years ago

  • Milestone 3.0 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

The function name is a clear reference to what the function manipulates - $GLOBALS.

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

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Your argument is not very convincing, the meaning of the function does not depend on case here.

This should have propper reasons, the coding standard is very clear about how functions have to be written:

Variables, functions, file names, and operators
Use lowercase letters in variable and function names (never camelcase). Separate words via underscores

Since this function is even called internally (in a single point of invoke!), I'll extend the patch to reflect that.

Please rething your argumentation.

@hakre
10 years ago

#6 @dd32
10 years ago

  • Owner set to dd32
  • Status changed from reopened to accepted

In this case, I do not think changing the function name would be suitable, GLOBALS is the functionality its affecting.

As for the variables, they should be renamed to stay within the coding standards

#7 @dd32
10 years ago

  • Milestone set to 3.0

#8 @dd32
10 years ago

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

(In [13598]) Coding Standards: no camelcase variables, use lowercase for variables. Props hakre. Fixes #12519

#9 @hakre
10 years ago

I'm against making exceptions. The function is well named regardless of it's case. All contributors should be treated equal.

This function is the only exception that I could find in the whole codebase (!), I see it problematic to show other developers that certain personal styles of certain developers are preferred for not obvious reason.

Background info: Function was moved in load.php in r12732. Originally it was introduce in wp-settings.php as unregister_GLOBALS() in r2784 and renamed in r4203 to it's current name wp_unregister_GLOBALS() to prevent name collisions with various functions called unregister_globals() in many other projects.

Changing the case of the function as I suggested has not negative inpact, only positive.

#10 @hakre
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I am pretty sure the function name is even misleading with uppercase letters.

The function is in there to turn over a PHP setting called register_globals. The case is "all lowercase". That setting is pretty well known.

The functionality of the function in question in this ticket is to "revert" the effect of a switched on register_globals PHP ini setting.

Next to this new information, even after the last commit, the function name still violates the wordpress coding standard.

The issue therefore is still unfixed.

#11 @nacin
10 years ago

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

#12 @hakre
10 years ago

Related to violating coding standard function naming: #13583

Note: See TracTickets for help on using tickets.