User "Catrope" changed the status of MediaWiki.r95272. Old Status: new New Status: ok
User "Catrope" also posted a comment on MediaWiki.r95272. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/95272#c21928 Commit summary: add support for writing out checkpoint files of xml dump at regular intervals (close and rename file based on filename pattern which includes first and last page id written) Comment: <pre> + $count = substr_count ($checkpointFile,"%s"); + if (substr_count ($checkpointFile,"%s") != 2) { </pre> Huh? :P You should use $count in the if here. <pre> + $filenameList = $this->egress->getFilename(); + if (! is_array($filenameList)) { + $filenameList = array( $filenameList ); + } </pre> You can also do this with <code>$filenameList = (array)$this->egress->getFilename();</code> , provided that the return value of that function isn't an object. Besides, it seems you don't even need it here: <code>count( 'a string' ) === 1</code> <pre> + # we wrote some stuff after last checkpoint that needs renamed */ </pre> Interesting comment style ;) <pre> + $firstPageID = str_pad($this->firstPageWritten,9,"0",STR_PAD_LEFT); + $lastPageID = str_pad($this->lastPageWritten,9,"0",STR_PAD_LEFT); </pre> Wouldn't you be able to do this inside the printf format string using <code>%09d</code> ? <pre> + $newFilenames[] = $fileinfo{'dirname'} . '/' . $checkpointNameFilledIn; </pre> I'm pretty sure the <code>{'dirname'}</code> syntax is deprecated in favor of <code>['dirname']</code> and I'm surprised it even worked: AFAIK the only use of braces that way was to get the n-th character of a string with <code>$str{2}</code> . OK otherwise, marking as such. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
