User:Salvatore Ingala/Review 1
(in progress)
Use OOP, Luke!
editStatic stuff like getGadgetPrefsDescription()
is simply creepy, why not $gadget->getPrefsDescription()
? Further points dduced for duplication: the same function uses Gadget::loadStructuredList()
, then iterates over the array, then does its job. Let's see the callers... bah, ApiGetGadgetPrefs does the same!
- Done in r90127. Salvatore Ingala 18:06, 15 June 2011 (UTC)
Gadgets_body.php
edit- This thingie had grown to the point where it could make sense to split it into several files.
- Done in r90469. Salvatore Ingala 16:48, 20 June 2011 (UTC)
- Profiling: add wfProfileIn()/wfProfileOut() wherever appropriate.
- Done in r90525. Added to the UserLoadOptions hook, it's probably the only relevant place in new code. Salvatore Ingala 14:13, 21 June 2011 (UTC)
- SQL injection via gadget name in getUserPrefs()/setUserPrefs()
- Why duplicate stuff from class User here at all?
- Done in r90215: now using UserLoadOptions and UserSaveOptions hooks instead of direct DB access. Salvatore Ingala 17:07, 16 June 2011 (UTC)
- Verify that all existing gadgets will survive wrapping into yet another closure.
- Once getGadgetPrefsDescription() has been turned into a member function, cache its result.
- Done in r90525. Salvatore Ingala 14:13, 21 June 2011 (UTC)
- serialize() is much faster than JSON for prefs storage.
- Done in r90322. Salvatore Ingala 21:59, 17 June 2011 (UTC)
ApiGetGadgetPrefs.php
editWhy not just merge it into ApiQueryGadgets?
jquery.formBuilder.js
edit- Rename function $s(), too close to "variable holding a jQuery object
".
- Done in r90466 Salvatore Ingala 15:48, 20 June 2011 (UTC)
Other
edit- I don't know how stuff works if there are no unit tests. It may work, but sometimes things work accidentally.
- Partially done in r90660 and r90702. Salvatore Ingala 09:42, 24 June 2011 (UTC)
- Don't use wfMsgInSaneNameThatIsAlsoLong()-like functions, there's Message class for new code.
- Done in r90525. Salvatore Ingala 14:13, 21 June 2011 (UTC)
- Use doxygen-compliant function documentation, see Manual:Coding conventions#Documentation.