r86367 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r86366‎ | r86367 | r86368 >
Date:23:20, 18 April 2011
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
Apply phase3 related Title fixes. With one minor addition, Title constructor marked as protected, "just in case"...

Patches by Yuvi Panda

From (bug 28583) Remove all /* private */ declarations in MediaWiki core
Modified paths:
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/SampleTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/parser/TagHooks.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/SampleTest.php
@@ -88,7 +88,7 @@
8989 * See http://www.phpunit.de/manual/3.4/en/appendixes.annotations.html#appendixes.annotations.expectedException
9090 */
9191 function testTitleObjectFromObject() {
92 - $title = Title::newFromText( new Title( "test" ) );
 92+ $title = Title::newFromText( Title::newFromText( "test" ) );
9393 $this->assertEquals( "Test", $title->isLocal() );
9494 }
9595 }
Index: trunk/phase3/tests/phpunit/includes/parser/TagHooks.php
@@ -21,7 +21,7 @@
2222 $parser = new Parser( $wgParserConf );
2323
2424 $parser->setHook( $tag, array( $this, 'tagCallback' ) );
25 - $parserOutput = $parser->parse( "Foo<$tag>Bar</$tag>Baz", new Title( 'Test' ), new ParserOptions );
 25+ $parserOutput = $parser->parse( "Foo<$tag>Bar</$tag>Baz", Title::newFromText( 'Test' ), new ParserOptions );
2626 $this->assertEquals( "<p>FooOneBaz\n</p>", $parserOutput->getText() );
2727
2828 $parser->mPreprocessor = null; # Break the Parser <-> Preprocessor cycle
@@ -36,7 +36,7 @@
3737 $parser = new Parser( $wgParserConf );
3838
3939 $parser->setHook( $tag, array( $this, 'tagCallback' ) );
40 - $parser->parse( "Foo<$tag>Bar</$tag>Baz", new Title( 'Test' ), new ParserOptions );
 40+ $parser->parse( "Foo<$tag>Bar</$tag>Baz", Title::newFromText( 'Test' ), new ParserOptions );
4141 $this->fail('Exception not thrown.');
4242 }
4343
@@ -48,7 +48,7 @@
4949 $parser = new Parser( $wgParserConf );
5050
5151 $parser->setFunctionTagHook( $tag, array( $this, 'functionTagCallback' ), 0 );
52 - $parserOutput = $parser->parse( "Foo<$tag>Bar</$tag>Baz", new Title( 'Test' ), new ParserOptions );
 52+ $parserOutput = $parser->parse( "Foo<$tag>Bar</$tag>Baz", Title::newFromText( 'Test' ), new ParserOptions );
5353 $this->assertEquals( "<p>FooOneBaz\n</p>", $parserOutput->getText() );
5454
5555 $parser->mPreprocessor = null; # Break the Parser <-> Preprocessor cycle
@@ -63,7 +63,7 @@
6464 $parser = new Parser( $wgParserConf );
6565
6666 $parser->setFunctionTagHook( $tag, array( $this, 'functionTagCallback' ), SFH_OBJECT_ARGS );
67 - $parser->parse( "Foo<$tag>Bar</$tag>Baz", new Title( 'Test' ), new ParserOptions );
 67+ $parser->parse( "Foo<$tag>Bar</$tag>Baz", Title::newFromText( 'Test' ), new ParserOptions );
6868 $this->fail('Exception not thrown.');
6969 }
7070
Index: trunk/phase3/includes/Title.php
@@ -78,9 +78,8 @@
7979
8080 /**
8181 * Constructor
82 - * @private
8382 */
84 - /* private */ function __construct() { }
 83+ protected function __construct() { }
8584
8685 /**
8786 * Create a new Title from a prefixed DB key

Follow-up revisions

RevisionCommit summaryAuthorDate
r86387Followup r86367, don't make constructor protected just yet, few underlying ca...reedy10:40, 19 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86335(bug 28583) chase down and beat to death external calls of functions marked /...happy-melon19:03, 18 April 2011
r86363Apply LoadBalancer related fixes...reedy23:12, 18 April 2011
r86365Apply Extension related new constructor fixes...reedy23:17, 18 April 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:21, 19 April 2011

You've got one: PHP Fatal error: Call to protected Title::__construct() from context 'Preferences' in /www/w/includes/Preferences.php on line 298

)
#Comment by MaxSem (talk | contribs)   07:59, 19 April 2011

You've also failed to grep extensions, this change breaks some of them. Suggest the protectization and using the usual deprecation process.

#Comment by Reedy (talk | contribs)   10:41, 19 April 2011

Remove the protected state for the moment. Not sure on a sane way to make it deprecated, as we don't want internal callers generating warnings etc...

#Comment by Happy-melon (talk | contribs)   10:45, 19 April 2011

stick a wfDeprecated() and a @since in the codepath and leave it for a couple of versions?

#Comment by Reedy (talk | contribs)   11:02, 19 April 2011

Which?

Looking around there is only one code instance of "new Title" in extensions, and that's in JS, so doesn't count

There are loads in comments, which are also irrelevant

The rest seem to be done in r86365

#Comment by Reedy (talk | contribs)   11:02, 19 April 2011

Which?

Looking around there is only one code instance of "new Title" in extensions, and that's in JS, so doesn't count

There are loads in comments, which are also irrelevant

The rest seem to be done in r86365

Status & tagging log