Ori.livneh has uploaded a new change for review.
https://gerrit.wikimedia.org/r/109850
Change subject: Make abstract Config class truly implementation-agnostic
......................................................................
Make abstract Config class truly implementation-agnostic
Some corrections to I13baec0b6 ("Config: Add Config and GlobalConfig classes"):
* Remove $prefix args from Config::set and ::get. The idea of having an
abstract Config class is to abstract some notion of configuration data from
the particular way in which it is currently implemented (global variables).
So the abstract base class has no business dealing with variable name
prefixes.
* Rename GlobalConfig to GlobalVarConfig, which makes it clearer that it isn't
referring to the scope of the configuration value, but to the scope of the
variable name which provides it.
* Removed Config::factory. One of the things we discussed at the summit was the
possibility of different extensions providing configuration data by different
means on a single MediaWiki instance, so the notion of a single, canonical
$wgConfigClass doesn't make sense.
* The fact that the unit test didn't care to check the Status object returned
by the setter is a good indication that the design needs to be rethought.
What's wrong with true / false? Do we really suppose that Config::set callers
need to have rich information about why attempting to set a configuration
variable failed?
Change-Id: I5a5857fcfa07598ba4ce9ae5bbb4ce54a567d31e
---
M includes/AutoLoader.php
M includes/config/Config.php
R includes/config/GlobalVarConfig.php
M tests/phpunit/includes/config/GlobalConfigTest.php
4 files changed, 14 insertions(+), 32 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/50/109850/1
diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 00191c3..f8597a7 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -407,7 +407,7 @@
# includes/config
'Config' => 'includes/config/Config.php',
- 'GlobalConfig' => 'includes/config/GlobalConfig.php',
+ 'GlobalVarConfig' => 'includes/config/GlobalVarConfig.php',
# includes/content
'AbstractContent' => 'includes/content/AbstractContent.php',
diff --git a/includes/config/Config.php b/includes/config/Config.php
index 067b1e4..b11381e 100644
--- a/includes/config/Config.php
+++ b/includes/config/Config.php
@@ -29,31 +29,13 @@
abstract class Config {
/**
- * @param string $name configuration variable name without prefix
- * @param string $prefix of the variable name
- * @return mixed
+ * @param string $name Name of configuration option
*/
- abstract public function get( $name, $prefix = 'wg' );
+ abstract public function get( $name );
/**
- * @param string $name configuration variable name without prefix
- * @param mixed $value to set
- * @param string $prefix of the variable name
- * @return Status object indicating success or failure
+ * @param string $name Name of configuration option
+ * @param mixed $value Value to set
*/
- abstract public function set( $name, $value, $prefix = 'wg' );
-
- /**
- * @param string|null $type class name for Config object,
- * uses $wgConfigClass if not provided
- * @return Config
- */
- public static function factory( $type = null ) {
- if ( !$type ) {
- global $wgConfigClass;
- $type = $wgConfigClass;
- }
-
- return new $type;
- }
+ abstract public function set( $name, $value );
}
diff --git a/includes/config/GlobalConfig.php
b/includes/config/GlobalVarConfig.php
similarity index 88%
rename from includes/config/GlobalConfig.php
rename to includes/config/GlobalVarConfig.php
index 1b1cd89..725f820 100644
--- a/includes/config/GlobalConfig.php
+++ b/includes/config/GlobalVarConfig.php
@@ -25,20 +25,20 @@
*
* @since 1.23
*/
-class GlobalConfig extends Config {
+class GlobalVarConfig extends Config {
/**
* @see Config::get
*/
public function get( $name, $prefix = 'wg' ) {
- return $GLOBALS[$prefix . $name];
+ return $GLOBALS[ $prefix . $name ];
}
/**
* @see Config::set
*/
public function set( $name, $value, $prefix = 'wg' ) {
- $GLOBALS[$prefix . $name] = $value;
- return Status::newGood();
+ $GLOBALS[ $prefix . $name ] = $value;
+ return true;
}
}
diff --git a/tests/phpunit/includes/config/GlobalConfigTest.php
b/tests/phpunit/includes/config/GlobalConfigTest.php
index b605a46..859089c 100644
--- a/tests/phpunit/includes/config/GlobalConfigTest.php
+++ b/tests/phpunit/includes/config/GlobalConfigTest.php
@@ -1,13 +1,13 @@
<?php
-class GlobalConfigTest extends MediaWikiTestCase {
+class GlobalVarConfigTest extends MediaWikiTestCase {
- /** @var GlobalConfig $config */
+ /** @var GlobalVarConfig $config */
protected $config;
protected function setUp() {
parent::setUp();
- $this->config = new GlobalConfig;
+ $this->config = new GlobalVarConfig;
}
public static function provideGet() {
@@ -23,7 +23,7 @@
* @param string $name
* @param array $params
* @dataProvider provideGet
- * @covers GlobalConfig::get
+ * @covers GlobalVarConfig::get
*/
public function testGet( $name, $params ) {
$rand = wfRandom();
--
To view, visit https://gerrit.wikimedia.org/r/109850
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5a5857fcfa07598ba4ce9ae5bbb4ce54a567d31e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
