https://bugzilla.wikimedia.org/show_bug.cgi?id=63224
Chris Steipp <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #5 from Chris Steipp <[email protected]> --- In general, please follow the style guide (unless Wikibase has it's own) The build directory shouldn't be web accessible-- is this meant to be removed in production? Otherwise, add an htaccess rule for it. The code uses PropertyId::newFromNumber, which looks like isn't supposed to be used. ./PropertySuggesterHooks.php * It seems like the JS doesn't need to be added on every page view ./maintenance/UpdateTable.php * Not a security issue, but using do { } while would make the code more readable. ./src/PropertySuggester/Suggesters/SimpleSuggester.php Line 75 - please escape $minProbability Please add profiling ./src/PropertySuggester/ResultBuilder.php Nitpick, but the "createJSON" function returns an array. Naming consistency makes reviewing easier. ./src/PropertySuggester/GetSuggestions.php The performance of using continue + limit as your db limit seems like it may cause issues. It's efficient, but running profiling will tell if it's a serious issue. ./src/PropertySuggester/UpdateTable/Importer/MySQLImporter.php * For production, imports need to be broken up into chunks. Talk with Sean, but Asher used to recommend importing in blocks of about 10k to avoid overloading the slaves. ./src/PropertySuggester/SuggesterParamsParser.php Line 48 - test is_null first ./PropertySuggester.php Please don't require_once /vendor/autoload.php -- 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
