User "Tim Starling" changed the status of MediaWiki.r84610. Old Status: new New Status: fixme
User "Tim Starling" also posted a comment on MediaWiki.r84610. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/84610#c21787 Commit summary: * Put parser output file version tracking to core * Added some ParserOutput accessors * A few cleanups to fetchFile() Comment: Comments on this project in general: I think you should provide an associative array of options to Parser::fetchFileAndTitle(), suitable for passing through to RepoGroup::findFile or RepoGroup::findFileFromKey(), instead of passing through the $time and $sha1 parameters separately. Also, I think instead of adding a $sha1 parameter to the BeforeParserMakeImageLinkObj hook (which you did in r84591), you should give a reference to the options array, and the use of the $time parameter should be deprecated. Then instead of the rather strange and undocumented convention you introduced here, allowing the hook to set $time to the string "0" to indicate that the image should not be shown, you should add an optional element to the options array specifically for that purpose. Similarly, the time and sha1 tracking introduced here to ParserOutput and OutputPage should be altered to track these options arrays instead. ParserOutput::getImageTimeKeys() could be replaced by ParserOutput::getImageSearchOptions(). The increment to Parser::VERSION is unnecessary and damaging and should be reverted. Instead, make FlaggedRevs gracefully handle the case where the relevant ParserOutput key is missing: have it use fr_fileSHA1Keys instead. Note that Parser::VERSION is meant to be the MediaWiki version where it was last changed. Presumably if you updated it now you would change it to 1.18.0. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
