openlp-core team mailing list archive

[Michael Gorven <michael@xxxxxxxxxxxxx>]

I would rather write convertStringToBoolean() like this:

    return stringvalue.lower() in ('true', 'on', 'yes', 'y')

+        words=words.replace(u'\r\n', u'\n')
+        verses_text = words.split(u'\n')

Rather use words.splitlines().

+        import time

Any reason why that's not imported at the top of the file?

+from renderer import  Renderer

Always use absolute imports (openlp.core.lib.renderer).

+        lines.append(u'Amazing Grace!')

I think that the preview text should either be loaded from a file, or stored 
as a variable at the top of the file.

Please use 'bzr mv' when renaming files to prevent breaks in revision history. 
(I'd prefer that your pluginmanager.py and renderer.py renames be fixed, but 
it probably isn't trivial.)

+    def setRenderManager(self, renderManager):
+        self.renderManager = renderManager

Don't use getter and setter methods. Properties can be used later on if it's 
necessary to control access to class variables.

I don't know how feasible it is, but it would be nice if branches/reviews were 
smaller.

-- 
https://code.launchpad.net/~trb143/openlp/ThemeManager2/+merge/5894
Your team openlp.org Core is subscribed to branch lp:openlp.