r98405 MediaWiki - Code Review archive

$context = RequestContext::getMain();

I don't consider this an acceptable replacement pattern for globals.

HistoryPage has access to an Article instance which should have access to a IContextSource, please attempt to get the context from there.

Please see some of my relevant pageoutput branch changes for reference: r1=97082&r2=97172 http://svn.wikimedia.org/viewvc/mediawiki/branches/pageoutput/includes/HistoryPage.php?r1=97082&r2=97172

Yes, it sucks. It's not a general pattern for use. Article needs a rewrite and HistoryPage should take a WikiPage and a Context anyway, which I'm not doing right now.

You can make the suggested change if you want of course.

	public static function newFromKey( $key, $type ) {
		$cache = new self();

		$cache->mKey = (string)$key;
		$cache->mType = (string)$type;
		$cache->mExt = 'cache';

		return $cache;
	}

new self(); in an abstract class isn't valid:

abstract class foo {
static function create() {
return new self();
}
}

class foo2 extends foo {

}

var_dump( foo::create() );

PHP Fatal error:  Cannot instantiate abstract class foo in /home/reedy/test.php on line 5
PHP Stack trace:
PHP   1. {main}() /home/reedy/test.php:0
PHP   2. foo::create() /home/reedy/test.php:13

abstract class foo {
static function create() {
return new self();
}
}

class foo2 extends foo {

}

var_dump( foo1::create() );

reedy@ubuntu64-web-esxi:~$ php test.php
PHP Fatal error:  Cannot instantiate abstract class foo in /home/reedy/test.php on line 5
PHP Stack trace:
PHP   1. {main}() /home/reedy/test.php:0
PHP   2. foo::create() /home/reedy/test.php:13

/* Why is there no gzfile_get_contents() or gzdecode()? */
return implode( '', gzfile( $this->cachePath() ) );

Not your code of course, but since you're claiming to have rewritten this class, I guess you get responsibility for this sort of thing. There is no gzdecode() in PHP 5.2, but there is readgzfile() which can be used with output buffering to get the file contents as a string without using large amounts of memory and CPU time by splitting it into an array. Alternatively gzopen() and a loop of gzread() can be used. gzopen() appears to return a stream resource, so stream_get_contents() would be another option.

r110871 implements gzopen() + stream_get_contents()

Giving to Antoine to peer review per conversation in MWCore meeting today.