openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing
In terms of consistent naming scheme, there is (for instance) "mainColourButtonClicked" and "mainColorButton" (see the missing "u"?). It's not a formal naming scheme, but I think consensus in IRC is that we use British English, i.e. Colour over Color.


Also, you have snippets of code (yes, *your* code, but if you're reformatting most of it, might as well do this too) like so:

QtCore.QObject.connect(self.backgroundComboBox,
    QtCore.SIGNAL(u'currentIndexChanged(int)'),
    self.onBackgroundComboBox)

Your method is called "on<Widget>" but should be called "on<Widget><Event>".


Line 504, you need a space after the comma.


Other than that, it looks good to me. Just gotta get those few things fixed up.
-- 
https://code.launchpad.net/~m2j/openlp/work/+merge/44748
Your team OpenLP Core is subscribed to branch lp:openlp.