r70701 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r70700‎ | r70701 | r70702 >
Date:14:55, 8 August 2010
Author:soxred93
Status:resolved (Comments)
Tags:
Comment:
Followup to r70638: Clean up code, add prop to function args, add comment
Modified paths:
  • /trunk/phase3/includes/api/ApiQueryPageProps.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiQueryPageProps.php
@@ -72,7 +72,7 @@
7373
7474
7575 foreach ( $this->everything as $pageid => $title ) {
76 - $pageInfo = $this->extractPageInfo( $pageid, $title );
 76+ $pageInfo = $this->extractPageInfo( $pageid, $title, $prop );
7777 $fit = $result->addValue( array(
7878 'query',
7979 'pages'
@@ -92,7 +92,7 @@
9393 * @param $title Title object
9494 * @return array
9595 */
96 - private function extractPageInfo( $pageid, $title ) {
 96+ private function extractPageInfo( $pageid, $title, $prop ) {
9797 global $wgPageProps;
9898
9999 $pageInfo = array();
@@ -121,7 +121,7 @@
122122 return $pageInfo;
123123 }
124124
125 - public function getCacheMode( $params ) {
 125+ public function getCacheMode() {
126126 return 'public';
127127 }
128128
@@ -148,6 +148,9 @@
149149 'continue' => 'When more results are available, use this to continue',
150150 );
151151
 152+ //This mess of code first gets the length of the biggest propname, and adds two onto it to make
 153+ //the number of characters should be used before the dash. If the biggest propname is shorter than 12 characters,
 154+ //the number of characters before the dash become 14.
152155 $maxLen = max( array_map( 'strlen', array_keys( $wgPageProps ) ) );
153156 $matchLen = $maxLen + 2;
154157 if( $maxLen < 12 ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r70770Followup to r70701: Facepalm fixing.soxred9316:26, 9 August 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r70638-(bug 24484) Add prop=pageprops module...soxred9318:50, 7 August 2010

Comments

#Comment by Catrope (talk | contribs)   15:03, 9 August 2010
-	public function getCacheMode( $params ) {
+	public function getCacheMode() {

Convention seems to be to keep the parameter around even if unused.

-			$pageInfo = $this->extractPageInfo( $pageid, $title );
+			$pageInfo = $this->extractPageInfo( $pageid, $title, $prop );

Doesn't help because $prop isn't set anywhere.

#Comment by X! (talk | contribs)   16:30, 9 August 2010

Now fixed.

#Comment by Bryan (talk | contribs)   19:16, 10 September 2010

The point of comments is not only to describe what is being done (I can see that it is adding 2 to the largest propname), but why it is being done. What it should mention is that you are doing this in order to properly align the properties in the help documentation.

Status & tagging log