r71441 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r71440‎ | r71441 | r71442 >
Date:20:55, 22 August 2010
Author:demon
Status:ok (Comments)
Tags:
Comment:
Make getSoftwareLink() static so I can use it without instantiating (and opening) a bunch of databases I probably can't support. Can't have an abstract parent now, so throw an exception for anyone who writes a child class without bothering to implement it. All of this to add some helpful text links in an infobox in the installer :)
Modified paths:
  • /trunk/phase3/includes/db/Database.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseIbm_db2.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseMssql.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseMysql.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseOracle.php (modified) (history)
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseSqlite.php (modified) (history)
  • /trunk/phase3/includes/installer/Installer.i18n.php (modified) (history)
  • /trunk/phase3/includes/installer/WebInstallerPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabaseMysql.php
——@@ -362,7 +362,7 @@
363363 return 'LOW_PRIORITY';
364364 }
365365
366 - function getSoftwareLink() {
 366+ public static function getSoftwareLink() {
367367 return '[http://www.mysql.com/ MySQL]';
368368 }
369369
Index: trunk/phase3/includes/db/DatabaseOracle.php
——@@ -817,7 +817,7 @@
818818 /**
819819 * @return string wikitext of a link to the server software's web site
820820 */
821 - function getSoftwareLink() {
 821+ public static function getSoftwareLink() {
822822 return '[http://www.oracle.com/ Oracle]';
823823 }
824824
Index: trunk/phase3/includes/db/DatabasePostgres.php
——@@ -1143,7 +1143,7 @@
11441144 /**
11451145 * @return string wikitext of a link to the server software's web site
11461146 */
1147 - function getSoftwareLink() {
 1147+ public static function getSoftwareLink() {
11481148 return "[http://www.postgresql.org/ PostgreSQL]";
11491149 }
11501150
Index: trunk/phase3/includes/db/DatabaseIbm_db2.php
——@@ -1436,7 +1436,7 @@
14371437 * Returns link to IBM DB2 free download
14381438 * @return string wikitext of a link to the server software's web site
14391439 */
1440 - public function getSoftwareLink() {
 1440+ public static function getSoftwareLink() {
14411441 return "[http://www.ibm.com/software/data/db2/express/?s_cmp=ECDDWW01&s_tact=MediaWiki IBM DB2]";
14421442 }
14431443
Index: trunk/phase3/includes/db/Database.php
——@@ -2077,7 +2077,9 @@
20782078 *
20792079 * @return String: wikitext of a link to the server software's web site
20802080 */
2081 - abstract function getSoftwareLink();
 2081+ static function getSoftwareLink() {
 2082+ throw new MWException( "A child class of DatabaseBase didn't implement getSoftwareLink(), shame on them" );
 2083+ }
20822084
20832085 /**
20842086 * A string describing the current software version, like from
Index: trunk/phase3/includes/db/DatabaseMssql.php
——@@ -731,7 +731,7 @@
732732 /**
733733 * @return string wikitext of a link to the server software's web site
734734 */
735 - function getSoftwareLink() {
 735+ public static function getSoftwareLink() {
736736 return "[http://www.microsoft.com/sql/ MS SQL Server]";
737737 }
738738
Index: trunk/phase3/includes/db/DatabaseSqlite.php
——@@ -434,7 +434,7 @@
435435 /**
436436 * @return string wikitext of a link to the server software's web site
437437 */
438 - function getSoftwareLink() {
 438+ public static function getSoftwareLink() {
439439 return "[http://sqlite.org/ SQLite]";
440440 }
441441
Index: trunk/phase3/includes/installer/Installer.i18n.php
——@@ -226,6 +226,11 @@
227227 'config-type-postgres' => 'PostgreSQL',
228228 'config-type-sqlite' => 'SQLite',
229229 'config-type-oracle' => 'Oracle',
 230+ 'config-type-info' => 'Mediawiki supports the following database systems:
 231+
 232+* $1 is the primary target for Mediawiki and is best supported ([http://www.php.net/manual/en/mysql.installation.php how to compile PHP with MySQL support])
 233+* $2 is a popular open souce database system as an alternative to MySQL ([http://www.php.net/manual/en/pgsql.installation.php how to compile PHP with PostgreSQL support])
 234+* $3 is a lightweight database system which is very well supported. ([http://www.php.net/manual/en/pdo.installation.php How to compile PHP with SQLite support], uses PDO)',
230235 'config-header-mysql' => 'MySQL settings',
231236 'config-header-postgres' => 'PostgreSQL settings',
232237 'config-header-sqlite' => 'SQLite settings',
Index: trunk/phase3/includes/installer/WebInstallerPage.php
——@@ -224,6 +224,13 @@
225225 $types = "<ul class=\"config-settings-block\">\n";
226226 $settings = '';
227227 $defaultType = $this->getVar( 'wgDBtype' );
 228+
 229+ $mysql = DatabaseMysql::getSoftwareLink();
 230+ $postgres = DatabasePostgres::getSoftwareLink();
 231+ $sqlite = DatabaseSqlite::getSoftwareLink();
 232+ $this->addHTML( $this->parent->getInfoBox(
 233+ wfMsg( 'config-type-info', $mysql, $postgres, $sqlite ) ) );
 234+
228235 foreach ( $this->parent->getVar( '_CompiledDBs' ) as $type ) {
229236 $installer = $this->parent->getDBInstaller( $type );
230237 $types .=

Follow-up revisions

RevisionCommit summaryAuthorDate
r71449Followup r71441, break up messages by type, iterate over known types. Expand ...demon21:52, 22 August 2010
r71762Move all abstract stuff that DatabaseBase children must implement to new inte...demon23:10, 26 August 2010

Comments

#Comment by Platonides (talk | contribs)   21:00, 22 August 2010

> Can't have an abstract parent now

Why?

I think that each line of config-type-info should be a different message, and you would be probably iterating among the available databases.

#Comment by Nikerabbit (talk | contribs)   05:48, 23 August 2010

> Why ?

Because abstract static just isn't allowed.

#Comment by Platonides (talk | contribs)   21:46, 23 August 2010

Was sorted in irc. I just didn't have E_STRICT warnings enabled.

#Comment by Simetrical (talk | contribs)   21:47, 26 August 2010

Better to make it return a generic string (can we stick the class name in it, or is that not available in inherited static methods?) than to throw an exception. Abstract methods will result in a fatal error at parse time, not call time, so it will be caught immediately if the class doesn't implement it. Throwing an exception means the error will only break a few random pages and might not be noticed for a while, which is unnecessarily obnoxious.

#Comment by Platonides (talk | contribs)   22:15, 26 August 2010

It can use $this->getType() to identify itself.

#Comment by Simetrical (talk | contribs)   22:21, 26 August 2010

From a static method?

#Comment by 😂 (talk | contribs)   22:25, 26 August 2010

That's one option. Another would be creating an interface, which can contain abstract static methods.

But creating an interface for one method is kind of silly I think.

#Comment by 😂 (talk | contribs)   21:44, 10 September 2010

No longer throws an exception since we added the DatabaseType interface and defined getSoftwareLink() there. Resetting to new.

Status & tagging log