openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

Review: Approve
In general this looks fine. Once again, a couple of minor points (mostly just style):

Why is there a ThemeXMLBuilder and a ThemeXMLParser... Parser is so small, is there a reason why you didn't combine the two into a ThemeXML class?

Just remember that we're using ' for strings, not " (seen in a few places)

No spaces between param, "=", and default value (see "footer" param): 
  def _get_extent_and_render(self, line, tlcorner=(0,0), dodraw=False, color=None, footer = False):

In the function above, "dodraw" - surely that can be changed to simply "draw" ?
-- 
https://code.launchpad.net/~trb143/openlp/ThemeManager/+merge/5014
Your team openlp.org Core is subscribed to branch lp:openlp.