r81255 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81254‎ | r81255 | r81256 >
Date:16:23, 31 January 2011
Author:demon
Status:ok (Comments)
Tags:
Comment:
Stupid references, stupid PHP. I blame Nikerabbit (fixing r81254)
Modified paths:
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/SkinTemplate.php
——@@ -1173,7 +1173,7 @@
11741174
11751175 // Copy in case this undocumented, shady hook tries to mess with internals
11761176 $revid = $this->mRevisionId;
1177 - wfRunHooks( 'SkinTemplateBuildNavUrlsNav_urlsAfterPermalink', array( $this, &$nav_urls, &$revid, &$revid ) );
 1177+ wfRunHooks( 'SkinTemplateBuildNavUrlsNav_urlsAfterPermalink', array( &$this, &$nav_urls, &$revid, &$revid ) );
11781178 }
11791179
11801180 if( $this->mTitle->getNamespace() != NS_SPECIAL ) {

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r81254Remove a bunch of useless $wgOuts. Just pass them like we should. Also make t...demon15:47, 31 January 2011

Comments

#Comment by Nikerabbit (talk | contribs)   17:25, 31 January 2011

I think the function to be called should be fixed instead (or alternatively update the docs and add release notes... but meh).

#Comment by 😂 (talk | contribs)   17:29, 31 January 2011

Passing $this by reference is never necessary in PHP5. Lots of hooks do it though.

I can already see the version_compare()s in extensions if we try to change it :\

#Comment by IAlex (talk | contribs)   19:36, 31 January 2011

Not necessarily, there's no problem if the callback uses the parameter by value and it's passed by reference in wfRunHooks().

#Comment by 😂 (talk | contribs)   13:21, 4 March 2011

Not really sure where there is to fixme here. I was just reverting part of r81254 to the status quo.

#Comment by Krinkle (talk | contribs)   13:33, 4 March 2011

r81254 by demon: 

-			wfRunHooks( 'SkinTemplateBuildNavUrlsNav_urlsAfterPermalink', array( &$this, &$nav_urls, &$revid, &$revid ) );
+			wfRunHooks( 'SkinTemplateBuildNavUrlsNav_urlsAfterPermalink', array( $this, &$nav_urls, &$revid, &$revid ) );

r81255 by demon (this revision): 

-			wfRunHooks( 'SkinTemplateBuildNavUrlsNav_urlsAfterPermalink', array( $this, &$nav_urls, &$revid, &$revid ) );
+			wfRunHooks( 'SkinTemplateBuildNavUrlsNav_urlsAfterPermalink', array( &$this, &$nav_urls, &$revid, &$revid ) );

Nothing changed, he self-reverted something from shortly before this commit.

Marking new instead of fixme, if the & should be removed I'd say someone should commit it or open a bug if it causes problems. The & wasn't introduced in this nor the previous revision.

Status & tagging log