r76616 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r76615‎ | r76616 | r76617 >
Date:06:25, 13 November 2010
Author:robla
Status:ok (Comments)
Tags:
Comment:
Followup to r76111. Making the result parameter on getThumbUrl optional
Modified paths:
  • /trunk/phase3/includes/filerepo/ForeignAPIRepo.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/ForeignAPIRepo.php
——@@ -196,7 +196,7 @@
197197 return $ret;
198198 }
199199
200 - function getThumbUrl( $name, $width=-1, $height=-1, &$result ) {
 200+ function getThumbUrl( $name, $width=-1, $height=-1, &$result=NULL ) {
201201 $data = $this->fetchImageQuery( array(
202202 'titles' => 'File:' . $name,
203203 'iiprop' => 'url|timestamp',

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r76111InstantCommons: API caching expires after a day. Now check for existing prese...hartman16:26, 5 November 2010

Comments

#Comment by Bryan (talk | contribs)   21:40, 1 December 2010

Are optional by-ref parameters allowed? A quick manual search does not reveal anything on this subject.

#Comment by Brion VIBBER (talk | contribs)   21:45, 1 December 2010

Seems fine; certainly built-in functions do it all the time (example: the third parameters on preg_match() to return matches).

I don't get errors or warnings even with strict and deprecated on.

#Comment by Bryan (talk | contribs)   22:00, 1 December 2010

But is this actually defined behaviour?

Still marking this as ok though, if we would be limited to defined behaviour in php...

#Comment by Brion VIBBER (talk | contribs)   00:12, 2 December 2010

Looks like we're good; "Note: As of PHP 5, default values may be passed by reference" is stashed at the bottom of the section on default function args in the online manual.

#Comment by Bryan (talk | contribs)   07:48, 2 December 2010

Thanks, I overlooked that yesterday.

Status & tagging log