r90385 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90384‎ | r90385 | r90386 >
Date:07:25, 19 June 2011
Author:aaron
Status:resolved (Comments)
Tags:
Comment:
* Added (and use) $format param to formatTimePeriod() to make output less verbose for ValidationStatistics_body.php
* Small w/s changes to FlaggedRevsStats.php
Modified paths:
  • /trunk/extensions/FlaggedRevs/dataclasses/FlaggedRevsStats.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/presentation/specialpages/reports/ValidationStatistics_body.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/maintenance/language/messageTypes.inc (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messageTypes.inc
——@@ -311,6 +311,7 @@
312312 'seconds-abbrev',
313313 'minutes-abbrev',
314314 'hours-abbrev',
 315+ 'days-abbrev',
315316 'filerevert-backlink',
316317 'filedelete-backlink',
317318 'delete-backlink',
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -3658,6 +3658,7 @@
36593659 'seconds-abbrev' => 's', # only translate this message to other languages if you have to change it
36603660 'minutes-abbrev' => 'm', # only translate this message to other languages if you have to change it
36613661 'hours-abbrev' => 'h', # only translate this message to other languages if you have to change it
 3662+'days-abbrev' => 'd', # only translate this message to other languages if you have to change it
36623663
36633664 # Bad image list
36643665 'bad_image_list' => 'The format is as follows:
Index: trunk/phase3/languages/Language.php
——@@ -3410,14 +3410,19 @@
34113411
34123412 /**
34133413 * @todo Document
3414 - * @param $seconds String
 3414+ * @param $seconds int|float
 3415+ * @param $format String Optional, one of ("avoidseconds","avoidseconds"):
 3416+ * If "avoidminutes" don't mention minutes if $seconds >= 1 hour
 3417+ * If "avoidseconds" don't mention seconds/minutes if $seconds > 2 days
34153418 * @return string
34163419 */
3417 - function formatTimePeriod( $seconds ) {
 3420+ function formatTimePeriod( $seconds, $format = false ) {
34183421 if ( round( $seconds * 10 ) < 100 ) {
3419 - return $this->formatNum( sprintf( "%.1f", round( $seconds * 10 ) / 10 ) ) . $this->getMessageFromDB( 'seconds-abbrev' );
 3422+ return $this->formatNum( sprintf( "%.1f", round( $seconds * 10 ) / 10 ) ) .
 3423+ $this->getMessageFromDB( 'seconds-abbrev' );
34203424 } elseif ( round( $seconds ) < 60 ) {
3421 - return $this->formatNum( round( $seconds ) ) . $this->getMessageFromDB( 'seconds-abbrev' );
 3425+ return $this->formatNum( round( $seconds ) ) .
 3426+ $this->getMessageFromDB( 'seconds-abbrev' );
34223427 } elseif ( round( $seconds ) < 3600 ) {
34233428 $minutes = floor( $seconds / 60 );
34243429 $secondsPart = round( fmod( $seconds, 60 ) );
——@@ -3425,23 +3430,31 @@
34263431 $secondsPart = 0;
34273432 $minutes++;
34283433 }
3429 - return $this->formatNum( $minutes ) . $this->getMessageFromDB( 'minutes-abbrev' ) . ' ' .
 3434+ return $this->formatNum( $minutes ) . $this->getMessageFromDB( 'minutes-abbrev' ) .
 3435+ ' ' .
34303436 $this->formatNum( $secondsPart ) . $this->getMessageFromDB( 'seconds-abbrev' );
3431 - } else {
 3437+ } elseif ( round( $seconds ) <= 2*86400 ) {
34323438 $hours = floor( $seconds / 3600 );
34333439 $minutes = floor( ( $seconds - $hours * 3600 ) / 60 );
3434 - $secondsPart = round( $seconds - $hours * 3600 - $minutes * 60 );
3435 - if ( $secondsPart == 60 ) {
3436 - $secondsPart = 0;
3437 - $minutes++;
 3440+ $secondsPart = floor( $seconds - $hours * 3600 - $minutes * 60 );
 3441+ $s = $this->formatNum( $hours ) . $this->getMessageFromDB( 'hours-abbrev' ) .
 3442+ ' ' .
 3443+ $this->formatNum( $minutes ) . $this->getMessageFromDB( 'minutes-abbrev' );
 3444+ if ( $format !== 'avoidseconds' ) {
 3445+ $s .= ' ' . $this->formatNum( $secondsPart ) .
 3446+ $this->getMessageFromDB( 'seconds-abbrev' );
34383447 }
3439 - if ( $minutes == 60 ) {
3440 - $minutes = 0;
3441 - $hours++;
 3448+ return $s;
 3449+ } else {
 3450+ $days = floor( $seconds / 86400 );
 3451+ $s = $this->formatNum( $days ) . $this->getMessageFromDB( 'days-abbrev' ) . ' ';
 3452+ if ( $format === 'avoidminutes' ) {
 3453+ $hours = floor( ( $seconds - $days * 86400 ) / 3600 );
 3454+ $s .= $this->formatNum( $hours ) . $this->getMessageFromDB( 'hours-abbrev' );
 3455+ } else {
 3456+ $s .= $this->formatTimePeriod( $seconds - $days * 86400, $format );
34423457 }
3443 - return $this->formatNum( $hours ) . $this->getMessageFromDB( 'hours-abbrev' ) . ' ' .
3444 - $this->formatNum( $minutes ) . $this->getMessageFromDB( 'minutes-abbrev' ) . ' ' .
3445 - $this->formatNum( $secondsPart ) . $this->getMessageFromDB( 'seconds-abbrev' );
 3458+ return $s;
34463459 }
34473460 }
34483461
Index: trunk/extensions/FlaggedRevs/dataclasses/FlaggedRevsStats.php
——@@ -97,13 +97,13 @@
9898
9999 $dataSet = array();
100100 $dataSet[] = array(
101 - 'frs_stat_key' => 'reviewLag-sampleStartTimestamp',
102 - 'frs_stat_val' => $reviewData['sampleStartTS'], // unix
103 - 'frs_timestamp' => $encDataTimestamp );
 101+ 'frs_stat_key' => 'reviewLag-sampleStartTimestamp',
 102+ 'frs_stat_val' => $reviewData['sampleStartTS'], // unix
 103+ 'frs_timestamp' => $encDataTimestamp );
104104 $dataSet[] = array(
105 - 'frs_stat_key' => 'reviewLag-sampleEndTimestamp',
106 - 'frs_stat_val' => $reviewData['sampleEndTS'], // unix
107 - 'frs_timestamp' => $encDataTimestamp );
 105+ 'frs_stat_key' => 'reviewLag-sampleEndTimestamp',
 106+ 'frs_stat_val' => $reviewData['sampleEndTS'], // unix
 107+ 'frs_timestamp' => $encDataTimestamp );
108108 // All-namespace percentiles...
109109 foreach( $rPerTable as $percentile => $seconds ) {
110110 $dataSet[] = array(
Index: trunk/extensions/FlaggedRevs/presentation/specialpages/reports/ValidationStatistics_body.php
——@@ -32,11 +32,12 @@
3333 }
3434
3535 # Is there a review time table available?
36 - if ( is_array( $pData ) && count( $pData ) ) {
 36+ if ( count( $pData ) ) {
3737 $headerRows = $dataRows = '';
3838 foreach ( $pData as $percentile => $perValue ) {
3939 $headerRows .= "<th>P<sub>" . intval( $percentile ) . "</sub></th>";
40 - $dataRows .= '<td>' . $wgLang->formatTimePeriod( $perValue ) . '</td>';
 40+ $dataRows .= '<td>' .
 41+ $wgLang->formatTimePeriod( $perValue, 'avoidminutes' ) . '</td>';
4142 }
4243 $css = 'wikitable flaggedrevs_stats_table';
4344 $reviewChart = "<table class='$css' style='white-space: nowrap;'>\n";
——@@ -50,18 +51,19 @@
5152 if ( $timestamp != '-' ) {
5253 # Show "last updated"...
5354 $wgOut->addWikiMsg( 'validationstatistics-lastupdate',
54 - $wgLang->date( $timestamp, true ),
55 - $wgLang->time( $timestamp, true )
 55+ $wgLang->date( $timestamp, true ),
 56+ $wgLang->time( $timestamp, true )
5657 );
5758 }
5859 $wgOut->addHtml( '<hr/>' );
5960 # Show pending time stats...
60 - $wgOut->addWikiMsg( 'validationstatistics-pndtime', $wgLang->formatTimePeriod( $pt ) );
 61+ $wgOut->addWikiMsg( 'validationstatistics-pndtime',
 62+ $wgLang->formatTimePeriod( $pt, 'avoidminutes' ) );
6163 # Show review time stats...
6264 if ( !FlaggedRevs::useOnlyIfProtected() ) {
6365 $wgOut->addWikiMsg( 'validationstatistics-revtime',
64 - $wgLang->formatTimePeriod( $mt ),
65 - $wgLang->formatTimePeriod( $mdt ),
 66+ $wgLang->formatTimePeriod( $mt, 'avoidminutes' ),
 67+ $wgLang->formatTimePeriod( $mdt, 'avoidminutes' ),
6668 $reviewChart
6769 );
6870 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r90386Fixed doc typo in r90385aaron07:28, 19 June 2011
r90387More doc typos from r90385...coding at 4AM :)aaron08:32, 19 June 2011
r90390Added back some rounding code lost in r90385aaron09:27, 19 June 2011
r90411Followup r90385: Add new message key here tooraymond18:50, 19 June 2011
r90915Added formatTimePeriod() tests for r90385 and made some fixesaaron22:32, 27 June 2011

Comments

#Comment by MaxSem (talk | contribs)   09:17, 19 June 2011

Breaks unit tests: 

LanguageTest::testFormatTimePeriod
formatTimePeriod() rounding (<1h)
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-1h 0m 0s
+0h 59m 59s

/home/ci/cruisecontrol-bin-2.8.3/projects/mw/source/tests/phpunit/languages/LanguageTest.php:52
/home/ci/cruisecontrol-bin-2.8.3/projects/mw/source/tests/phpunit/MediaWikiTestCase.php:64
/home/ci/cruisecontrol-bin-2.8.3/projects/mw/source/tests/phpunit/MediaWikiPHPUnitCommand.php:20
/home/ci/cruisecontrol-bin-2.8.3/projects/mw/source/tests/phpunit/phpunit.php:60
#Comment by Aaron Schulz (talk | contribs)   09:27, 19 June 2011

r90390

#Comment by 😂 (talk | contribs)   19:19, 19 June 2011

Could use some unit tests of its own :)

Status & tagging log