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. _______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : [email protected] Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
