https://bugzilla.wikimedia.org/show_bug.cgi?id=16950





--- Comment #15 from Aryeh Gregor <[email protected]>  2009-04-21 
01:26:14 UTC ---
-        * @param $type String: A log type ('upload', 'delete', etc)
+        * @param $type Array: Log types ('upload', 'delete', etc)

"String or array" is more correct.

+               // If $type is not an array, make it an array
+               if ( !is_array( $type ) )
+               {
+                       $type=array( "$type" );
+               }

This should just be:

                $type = (array)$type;

Also note some style issues here (opening brace is on the same line as the if,
"=" should have spaces around it).

Also, $type should probably be renamed $types, it's a little confusing now.

-               if( isset($wgLogRestrictions[$type]) &&
!$wgUser->isAllowed($wgLogRestrictions[$type]) ) {
+               foreach ( $type as $i => $t ) {
+                               if( isset( $wgLogRestrictions[$t] ) &&
!$wgUser->isAllowed($wgLogRestrictions[$t]) ) {
+                                       unset( $type["$i"] );
+                               }

You seem to have too much indentation here.  $type["$i"] seems a bit odd to me,
may as well just be $type[$i].  Also, maybe best to avoid the extra $i and do
$type = array_diff( $type, array( $t ) ) instead of the unset.  If $type were
renamed $types, you could do foreach ( $types as $type ) instead of the
slightly confusing $t.

+               if ( count($type)==0 ) {

Style:

                if ( count( $type ) == 0 ) {

We use lots of spaces . . .

-                                               'type' => 'delete',
+                                               'type' => '',

-                                       array( 'type' => 'delete', 'page' =>
$this->mTitle->getPrefixedText() ) 
+                                       array( 'type' => '', 'page' =>
$this->mTitle->getPrefixedText() ) 

Is there a reason you're doing 'type' => '' instead of just dropping type
altogether?  Does that not work?

-'recreate-deleted-warn'            => "'''Warning: You are recreating a page
that was previously deleted.'''
+'moveddeleted-notice'              => 'This page has been moved and/or
deleted.
+The move and deletion log for the page are provided below for reference.',
+'recreate-moveddeleted-warn'       => "'''Warning: You are recreating a page
that was previously moved and/or deleted.'''

Moving without redirect is very unlikely compared to deletion, so I think it
would be best to keep these saying just "deleted" instead of "moved and/or
deleted".  If you're going to keep both, I'd say "deleted and/or moved without
redirect", which a) puts the common case first and b) makes it clear when this
actually shows up (you won't see it for pages that were moved normally and not
deleted).

(Why did you change the order of these two messages?)

I don't see any other problems.  With these fixed, I'd be willing to commit it,
if no problems show up when I test it out.


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
Wikibugs-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l

Reply via email to