User:Catrope/Extension review/Translate/old

Base revision: r95391

Translate.php

 * The  global functions should be in a static class, e.g.
 * What does  do? The way it interacts with the   is not really documented or intuitive
 * isn't exactly a good memcached key. It should be something unique and descriptive, e.g.

translate.sql

 * The index on  is completely useless given that the primary key is
 * What is this table for?

TranslateUtils.php

 * Line 49: can't you just use  or something along those lines?
 * Line 132: you're mixing ASC and DESC in an ORDER BY, and one of the fields you're ordering by is a derivative field (result of a function). That in combination with the large size of the recentchanges table on most WMF wikis (5M rows on enwiki) means this feature will have to be behind $wgMiserMode or otherwise disableable
 * Line 136: what batch existence query?
 * Line 139: you can also use
 * Line 170: you may want to use a strict comparison here, otherwise  will produce strange results
 * Line 202, 207: ZOMG, hardcoded English. Of all extensions... :)
 * Line 256, 280: what is so performance-sensitive here that you have to factor out a simple function call? That makes little sense to me

TranslateTasks.php

 * Line 361:  is not escaped
 * Line 462-468: can't you just use  here?

TranslateEditAddons.php

 * Generally, you should mention what hook each function is hooked into in a doc comment, that makes understanding the code easier without having to go back to look up the $wgHooks assignments each time
 * Line 44, 49: you're using strtr here to replace spaces/underscores and str_replace elsewhere. Also, don't you have a function lying around that does lowercasing and space/underscore stuff?
 * Line 53: -666 ?!?!?
 * Line 98: why would $next equal true?
 * Line 111: what is SkinChihuahua?
 * Line 122: $object seems to be an EditPage, document that
 * Line 193: eww, onclick
 * Line 254-258: what does this logic do and why? There are no comments
 * Line 294: don't pass an array as the second parameter to . The function supports this by accident only
 * Line 296: don't you really just mean to check that the second query didn't return zero results? If you meant it, say it, with something like ,   ,   or whatever. As currently written this function will return true in the (admittedly strange) case where 'fuzzy' isn't in the revtag_type table because   is true

utils/TranslationHelpers.php

 * Line 1224: ??? makeVariablesScript shouldn't be used this way. You should export variables through a MakeGlobalVariablesScript hook, and you shouldn't just export the return value of wfMsg, use the normal message exporting system instead