openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #00032
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