r78503 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r78502‎ | r78503 | r78504 >
Date:18:28, 16 December 2010
Author:yaron
Status:deferred (Comments)
Tags:
Comment:
Fixed bug in loadMessages()
Modified paths:
  • /trunk/extensions/ApprovedRevs/ApprovedRevs_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ApprovedRevs/ApprovedRevs_body.php
——@@ -206,6 +206,7 @@
207207
208208 public static function loadMessages() {
209209 // for backwards compatibility
 210+ global $wgVersion;
210211 if ( version_compare( $wgVersion, '1.16', '<' ) ) {
211212 wfLoadExtensionMessages( 'ApprovedRevs' );
212213 }

Comments

#Comment by Zakgreant (talk | contribs)   19:00, 16 December 2010

For cases like this, why wouldn't we use the $GLOBALS superglobal?

e.g.

if ( version_compare( $GLOBALS['wgVersion'], '1.16', '<' ) ) {

...

Explicit use of $GLOBALS['wgXXX'] makes it very clear that we're accessing the variable in the right scope.

#Comment by Yaron K. (talk | contribs)   16:56, 17 December 2010

Hi Zak,

I don't know what the difference is between "$GLOBALS['wgXXX']" and "$wgXXX"; but in any case, I'm just following MediaWiki convention.

#Comment by Zakgreant (talk | contribs)   18:41, 17 December 2010

Hi Yaron,

I see people doing both in the code and don't see anything about using the global keyword over the $GLOBALS array superglobal in http://www.mediawiki.org/wiki/Manual:Coding_conventions. Did I miss some coding conventions somewhere?

#Comment by Krinkle (talk | contribs)   18:47, 17 December 2010

Not all conventions are documented. Becuase of it I don't think it's a big problem to do it either way, but afaik the 'global' keyword is the most common (atleast in the core).

A possible argument I could think of for doing this is for easy scanning (either by humans or bots) to see which globals are used inside a function, without having to read the entire function.

#Comment by Yaron K. (talk | contribs)   21:38, 17 December 2010

Well, it might not be a convention, but it's certainly not a convention the other way either. Unless that changes, I won't be changing it in my code.

Status & tagging log