openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #20007
Re: [Merge] lp:~arjan-i/openlp/images_groups into lp:openlp
Review: Needs Fixing
This needs fixing:
1367, 1379, 1895, 2095
You should drop the len(). Instead of
if len(my_list) > 0:
do
if my_list:
1851 + if imageFile.group_id is 0:
Should be:
if imageFile.group_id == 0:
This *should* also be fixed (in the future):
Everything should be words_with_underscore. And if you change old code with still uses camelCase then it should be updated.
This *would* be nice to be fixed:
Importing old settings still does not work 100% correctly (trunk does also not work correctly). It would be good if you could look at this (ONLY if you WANT to). I can send you old config files (or check our dropbox if you are a member).
The "should" and "would" parts are not mandatory for me to approve this (particular, because you are "new" to OpenLP development and I don't know how motivated you are. I don't want to scare off people.)
Anyway, looks good :)
--
https://code.launchpad.net/~arjan-i/openlp/images_groups/+merge/153826
Your team OpenLP Core is subscribed to branch lp:openlp.
References