"Awjrichards" changed the status of Wikimedia.r984 to "fixme" and commented it.
URL: http://www.mediawiki.org/wiki/Special:Code/Wikimedia/984#c27680

Old Status: new
> New Status: fixme

Commit summary for Wikimedia.r984:

Adding inital version of the scripts for initial CR. Mostly complete.

Awjrichards's comment:

Yes, this is super nitpicky. But, you can do:
<pre>
microtime( true );
</pre>
To get the same exact thing as:
<pre>
+function getTime() {
+       $a = explode ( ' ', microtime() );
+       return ( double ) $a[ 0 ] + $a[ 1 ];
+}
</pre>
The only reason I bring this up is because it's a lot easier to read, which 
makes reviewers, devs inherting your code, and you 6 months from now much saner 
:)

Please add comments to the functions to make clear what they do and their 
expected inputs.

Don't forget to remove this debug output:
<pre>
+print $unsub_link . "\n";
</pre>

To be on the safe side, please escape BATCH_SIZE:
<pre>
+       # TODO: implement a priority?
+       $query = "
+               SELECT *
+               FROM `emails_tosend`
+               WHERE `sent` IS NULL
+               LIMIT " . BATCH_SIZE . "
+       ";
</pre>
You could do something like run it through intval().

<pre>
        if ( !$result ) {
                log_out( "FAILED" );
                log_out( mysql_error( $db ) );
                # TODO: die? break? throw?
        }
</pre>
That's a todo you should probably do :)

_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview

Reply via email to