[geeklog-devel] Static Pages Plugin and advanced editor

Dirk Haun dirk at haun-online.de
Fri Apr 30 16:08:44 EDT 2010


Samuel Leathers wrote:

>Please look it over when you get the chance and give me
>any feedback regarding coding style or functionality.

Haven't check the functionality, but here are some comments on the patch
itself:

- Please use unified diffs[1], i.e diff -u, since that provides some
context and improves chances that the patch can be applied when the line
numbers change

- Our Coding Guidelines[2] ask for the opening { to go at the end of a line

- When requesting only one value from the db, instead of DB_query
("SELECT advanced_editor FROM {$_TABLES['userprefs']} WHERE uid = {$_USER
['uid']};"); use

    DB_getItem($_TABLES['userprefs'], 'advanced_editor', "uid = {$_USER
['uid']}");

- You don't actually need to do that, though, since $_USER
['advanced_editor'] already contains that value (even for the Anonymous user)

Sorry for finding so much to criticise for so little code ;-) You may
want to open an issue in our bugtracker for that, btw, as it's a bit
late to include it in 1.7.0 and we don't want it to get lost.

Thanks for your efforts!

bye, Dirk

[1] http://wiki.geeklog.net/index.php/Submitting_Patches
[2] http://wiki.geeklog.net/index.php/Coding_Guidelines


-- 
http://www.geeklog.net/
http://geeklog.info/




More information about the geeklog-devel mailing list