https://bugzilla.wikimedia.org/show_bug.cgi?id=34778
--- Comment #20 from Platonides <platoni...@gmail.com> 2012-06-08 22:23:24 UTC --- Adding the link to the bug report so I don't need to go looking for it each time http://mementoweb.org/tools/wiki/memento.zip It would be easier to check if the changes were available in some repository. * No need of mmSetupExtension, set $wgHooks in the global scope. > $title = preg_replace('/ /', "_", $objTitle->getPrefixedText()); Used in several places. preg_replace() here is overkill, as str_replace() would do it. But in this case you'd just want $objTitle->getPrefixedURL() * Coding conventions: Tabs instead of spaces, spaces into brackets... * No need to make the SELECT rev_id rev_timestamp FROM revision a DISTINCT one, as those combined fields are always unique (the db server is probably realising and ignoring the distinct, but no need to add it). * Generation of the link header is wrong. Eg. in the way it is generating the article url. You're not doing any rawurlescaping, so some specially crafted article names could confuse memento clients. * Minor: give the author names in $wgExtensionCredits in an array. * Unneded space indenting in timegate/timegate.alias.php, timegate/timegate.i18n.php, timemap/timemap.alias.php, timemap/timemap.i18n.php and top of timemap/timemap.php > if( stripos($par, $wgServer.$waddress) == 0 ) { > $title = preg_replace( "|".$wgServer.$waddress."|", "", $par ); Wrong check and wrong regex. > $dbr->begin(); There is never a corresponding commit() or rollback(). You're doing an exit in the bottom, the php driver might be closing the transaction or even the connection, but don't rely on it. (several places) * wfLoadExtensionMessages() is deprecated since 1.15, expected to be removed in 1.20 (just remove that line). * Variable $wgMementoReqDateTime defined as global in execute(), set as local variable in tgParseRequestDateTime() and never used. * tgParseRequestDateTime() should use wfTimestamp() instead of strtotime() -> date() * When you have a title object, you don't need a manual query to revision table to get the latest revision, just use getLatestRevID(). * You have repeated code for selecting the first/prev/next of a revision. I think it could be abstracted in a single function. * TimeGate methods use a tg prefix that isn't really needed (you're scoped by the class name). * At TimeGate::tgGetMementoForResource if there's no revision for the given memento, it will merrily use the undefined variable $memRevUnixTS generating wrong SQL. Maybe you wanted to abort with an error message if there's no suitable memento? (even with the $oldestRevUnixTS / $recentRevUnixTS, there could be a race condition) It's in better shape than the previous version :) -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug. You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
