"Tim Starling" posted a comment on MediaWiki.r106752. URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/106752#c28188
Commit summary for MediaWiki.r106752: Merged FileBackend branch. Manually avoiding merging the many prop-only changes SVN likes to sprinkle in (easy to spot from the change list). Did not add SwiftFileBackend.php as it still is in development. Tim Starling's comment: The documentation of $wgLocalFileRepo needs to be updated. I'd like to see less code in Setup.php. The filerepo setup has grown to 143 lines, or 25% of the file. Calling FileBackendGroup/LockManagerGroup::singleton() from Setup.php seems slow and unnecessary. For many requests, a backend object will never be needed. You could remove the register() functions and configure the group from singleton() instead, like in RepoGroup::singleton(). The configuration backwards compatibility code could be moved to either singleton() or the group constructor. <pre> // File where directory should be $op = array( 'op' => 'delete', 'src' => $thumbDir ); $this->repo->getBackend()->doOperation( $op ); </pre> I'm sure this felt really elegant at the time, but I think at some point you're going to realise how much typing it is and regret doing it. Or if you don't regret it, other developers will just silently resent you forever. I suggest adding a few convenience wrappers for common operations, so that code like this will be shorter. You already used the short names such as FileBackend::delete() for internal interfaces, which makes things a bit awkward. It probably makes sense to rename the internal interfaces, allowing you to use the shortest possible names for the interface with FileRepo. I am thinking: <pre> $this->repo->getBackend()->delete( array( 'src' => $thumbDir ) ); </pre> Or maybe even: <pre> // File where directory should be $this->repo->getBackend()->delete( $thumbDir ); </pre> _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
