https://bugzilla.wikimedia.org/show_bug.cgi?id=15227
--- Comment #10 from Roan Kattouw <[email protected]> 2009-01-23 22:52:09 UTC --- (In reply to comment #7) > * The ApiQueryImageInfo::allProps() thing is confusing, unnecessary and > non-standard (other modules also have lots of props, and they don't put them > in > a separate function either). Indentation is more than enough of an aid to > distinguish the array of props from the rest of the getAllowedParams() array Whoops, I see now that it does have a use. > * What's the status of the FIXME if the stashed upload section? Seems to have disappeared in the branch. > * Also, if you want the comment parameter to default to '', just define that > default in getAllowedParams(). The amfilter parameter is a good example of how > to define defaults for string parameters > * It's probably cleaner to define $request = $this->getMain()->getRequest() > all > the way on top of execute(), since you're using the long version near there > too > * The permissions check should be moved all the way up, before validating > uploads that aren't allowed anyway Did these in the branch in r46100. > * Permissions are currently checked in two places. Also, if > verifyPermissions() > is sane, it'll check for isAllowed() too and return some sort of value > indicating why permission was denied (like Title::getUserPermissionsErrors()) This is related to what Tim said in bug 14925 comment #11. All these functions checking stuff should be merged into one big function that returns an error array (message key + params) that can be fed straight into dieUsageMsg() and into an OutputPage method whose name I don't remember. I recommend taking the rest of Tim's advice in that comment as well. > * Using dieUsage() for some errors and <upload result="Failure"> is > inconsistent. You should only use result="Failure" when you actually have more > information to convey. Also, the message map has an 'unknownerror' message, > but > you should really just dieUsageMsg() the error and let dieUsageMsg() discover > it's not in the message map and switch to 'unknownerror', that makes adding > messages easier > I could go and fix up all the dieUsage() calls, but that's kind of pointless if the upload code is gonna be rewritten to return error arrays anyway. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email ------- 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
