r82366 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r82365‎ | r82366 | r82367 >
Date:00:48, 18 February 2011
Author:jeroendedauw
Status:reverted (Comments)
Tags:
Comment:
rem not needed die() and made vars actually private
Modified paths:
  • /trunk/phase3/includes/ImagePage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ImagePage.php
——@@ -1,8 +1,5 @@
22 <?php
33
4 -if ( !defined( 'MEDIAWIKI' ) )
5 - die( 1 );
6 -
74 /**
85 * Special handling for image description pages
96 *
——@@ -10,10 +7,10 @@
118 */
129 class ImagePage extends Article {
1310
14 - /* private */ var $img; // Image object
15 - /* private */ var $displayImg;
16 - /* private */ var $repo;
17 - /* private */ var $fileLoaded;
 11+ private $img; // Image object
 12+ private $displayImg;
 13+ private $repo;
 14+ private $fileLoaded;
1815 var $mExtraDescription = false;
1916 var $dupes;
2017

Follow-up revisions

RevisionCommit summaryAuthorDate
r82462Partial revert my accidental revert of r82366 in r82445.maxsem16:25, 19 February 2011

Comments

#Comment by 😂 (talk | contribs)   00:51, 18 February 2011

Did you grep extensions for things possibly using these vars?

I'm firmly +1 for limiting scope when we can, but it's easy to miss them (especially with a generic name like $img :))

#Comment by 😂 (talk | contribs)   01:01, 18 February 2011

$fileLoaded and $repo seem to be ok (although the latter is difficult to grep for, there's some stuff using FileRepo-related code, and $repo in CodeReview is completely unrelated)

$displayImg is used in ImageTagging, but that should be a trivial fix.

$img has so much noise that I'm not even bothering to dig through those results yet.

#Comment by MaxSem (talk | contribs)   11:07, 19 February 2011

Fatal error: Cannot access private property ImagePage::$img in C:\Projects\MediaWiki\extensions\ProofreadPage\ProofreadPage_body.php on line 477

#Comment by MaxSem (talk | contribs)   11:15, 19 February 2011

This particualr error has been fixed in r82443, however I still suggest reverting.

#Comment by MaxSem (talk | contribs)   16:33, 19 February 2011

Accidentally reverted in r82445, we should decide which version should stick. Frankly, I would prefer to keep them public until 1.18 is branched to ensure b/c. There are stil a couple of uses in extensions/ which I'm gonna fix now.

#Comment by Platonides (talk | contribs)   16:37, 19 February 2011

Seems good keeping them public and unused at 1.18.

Status & tagging log