← Back to team overview

openlp-core team mailing list archive

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