MediaWiki r106813 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r106812‎ | r106813 (on ViewVC)‎ | r106814 >
Date:17:04, 20 December 2011
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Refactored batchAntiSpoof.php to subclass Maintenance

For part of bug 28747
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r106816Followup r106813, make batchRecord protected not private...reedy17:14, 20 December 2011

Past revisions this follows-up on

Rev.Commit summaryAuthorDate
r106809Followup r106808...reedy16:29, 20 December 2011

Comments

#Comment by Catrope (talk | contribs)   20:25, 21 December 2011
+		$this->batchRecord( $items );

Shouldn't this be in the if statement where you're clearing $items? Otherwise only the last batch will be recorded.

#Comment by Reedy (talk | contribs)   00:28, 22 December 2011

It is?

+			if ( $n % $batchSize == 0 ) {
+				$this->batchRecord( $items );
+				$items = array();
+			}
<?php
// Go through all usernames and calculate and record spoof thingies

$IP = getenv( 'MW_INSTALL_PATH' );
if ( $IP === false ) {
	$IP = dirname( __FILE__ ) . '/../..';
}
require( "$IP/maintenance/Maintenance.php" );

class BatchAntiSpoof extends Maintenance {

	/**
	 * @param $items array
	 */
	private function batchRecord( $items ) {
		SpoofUser::batchRecord( $items );
	}

	/**
	 * Do the actual work. All child classes will need to implement this
	 */
	public function execute() {
		$dbw = $this->getDB( DB_MASTER );

		$dbw->bufferResults( false );

		$batchSize = 1000;

		$result = $dbw->select( 'user', 'user_name', null, __FUNCTION__ );
		$n = 0;
		$items = array();
		foreach( $result as $row ) {
			if ( $n++ % $batchSize == 0 ) {
				$this->output( "...$n\n" );
			}

			$items[] = new SpoofUser( $row->user_name );

			if ( $n % $batchSize == 0 ) {
				$this->batchRecord( $items );
				$items = array();
			}
		}

		$this->batchRecord( $items );
		$this->output( "$n user(s) done.\n" );
	}
}

$maintClass = "BatchAntiSpoof";
require_once( DO_MAINTENANCE );
#Comment by Catrope (talk | contribs)   16:28, 9 January 2012
+		$result = $dbw->select( 'user', 'user_name', null, __FUNCTION__ );

Shouldn't you use __METHOD__ ?

Fine otherwise, marking OK.

Status & tagging log

  • 09:11, 18 January 2012 Siebrand (talk | contribs) changed the tags for r106813 [removed: tools]
  • 16:28, 9 January 2012 Catrope (talk | contribs) changed the status of r106813 [removed: new added: ok]
  • 17:41, 5 January 2012 MarkAHershberger (talk | contribs) changed the tags for r106813 [added: tools]
  • 00:28, 22 December 2011 Reedy (talk | contribs) changed the status of r106813 [removed: fixme added: new]
  • 20:25, 21 December 2011 Catrope (talk | contribs) changed the status of r106813 [removed: new added: fixme]