← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~trb143/openlp/ThemeManager2 into lp:openlp

 

Here's some comments (of the nit-picky variety - it's great to see this amount of progress!):

In string to boolean - does the string want "stripping" first, otherwise whitespace at the ends will cause it to be false?

In render_lines() the second parameter would be better renamed as "footer" (as elsewhere) and then the  docstring could be updated

In the theme, I see the background can be an image with transparent property set - have you tried that with a transparent BG - does it work?

Have we standardised on the US-english spelling of colour/color?  This code has color in, but I'm sure there are others with colour in - i think we ought to standardise on one or the other throughout. (I learned to program on an Apple II, so I can cope with "color" :)

I think the test_plugin_manager won't work run with the "import plugin_manager" above  the path.insert() call.

amendthemeform.paintUI() has a "print theme" line  - probably ought to be a log.debug?

In the mainwindow:
   self.plugin_helpers[u'theme'] = self.ThemeManagerContents # Theme manger
That comment is probably superfluous :)
-- 
https://code.launchpad.net/~trb143/openlp/ThemeManager2/+merge/5894
Your team openlp.org Core is subscribed to branch lp:openlp.



References