Code review comment for lp:~trb143/openlp/servicing

Revision history for this message
Martin Thompson (mjthompson) wrote :

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 :)

review: Approve

« Back to merge proposal