User "Awjrichards" changed the status of MediaWiki.r94720.

Old Status: new
New Status: fixme

User "Awjrichards" also posted a comment on MediaWiki.r94720.

Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/94720#c21103
Commit summary:

In the process of adding an API to ContributionTracking, I ended up refactoring 
the majority of the extension.
Many changes and additions, including the new API, a jquery module that uses 
the API, and an unlisted sysop-only API testing page.

Comment:

Sylistic point:
<pre>
static function mergeArrayDefaults( $params, $defaults, $nullify=false ) {
+               foreach ( $defaults as $key => $value ) {
+                       if ( array_key_exists( $key, $params ) ) {
+                               $defaults[$key] = $params[$key];
+                       }
+                       if ( $nullify && $defaults[$key] === '' ) {
+                               $defaults[$key] = null;
+                       }
+               }
+               return $defaults;
+       }
</pre>
It would be clearer if you reassigned $defaults to a variable other than 
$defaults, since as soon as you start modifying the values in $defaults, they 
are no longer default values.

In ContributionTracking_body.php, you changed the msg() function to:
<pre>
+       function msg() {
+               return wfMsgExt( func_get_arg( 0 ), array( 'escape', 'language' 
=> $this->lang ) );
        }
</pre>
But left msg() in ContributionTracking.processor.php with the $key argument.  
Was this on purpose?  Similarly, the msgWiki() functions also have the $key 
argument.  Also on purpose, or should they be changed?  Iirc, there was a good 
reason why you took the explicit arg out and started using func_get_arg().

Otherwise, this looks great - this is heading in a good direction!

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

Reply via email to