← Back to team overview

openlp-core team mailing list archive

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

 

Review: Approve
Possible tweaks?

Line 69 of diff - would the uninit'ed theme be better as None than u'' -  more pythonic? At line 437, it does seem to be that way
Line 113 - extra space after comma

Lines:
158	+ self.service_theme = self.ThemeComboBox.currentText()
159	+ self.RenderManager.set_service_theme(self.service_theme)
160	+ self.config.set_config(u'theme service theme', self.service_theme)

Do all these three have to be kept in sync like this?  It feels a bit potentially "fragile" (to me), but I guess if it's only ever going to be done this once, it won't matter!

Line 277: Would it be acceptable to use double-quoted strings to avoid escaping all the single quotes (this is the reason I default to double-quoted strings in my code :)

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



References