User "Krinkle" posted a comment on MediaWiki.r93654.

Full URL: 
https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Special:Code/MediaWiki/93654#c20653
Commit summary:

Follows-up r93515 CR: Whitespace

Comment:

For loops don't have their own context, there's no difference between
<source lang="javascript">
var foo = 'bar', len = 10;
for ( var n = 0; n < len; n++ ) {
 
}
</source>
and
<source lang="javascript">
var foo = 'bar';
for ( var n = 0, len = 10; n < len; n++ ) {
 
}
</source>
I agree that looking at what it actually does though, we don't need the loop at 
all actually (originally introduced in r70220, merged to trunk in r72349). But 
I don't think we need concatenation or appending either, since we're not 
actually dealing with two arrays. We're dealing with one array and one string 
(which for the purpose of appending was converted to an array (dependencies = 
[dependencies])).
The simplest solution is probably to clone the dependencies array and unshift 
the string to the beginning of it.
<source lang="javascript">
        var modulename = dependencies;
        if ( modulename in registry ) {
                // If called with a single module name, get it's dependencies 
as well
                dependencies = registry[modulename].dependencies.slice( 0 ); // 
clone, keep original object in tact
                dependencies.unshift( modulename );
        }
</source>
What do you think ?

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

Reply via email to