openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #00058
Re: [Merge] lp:~trb143/openlp/servicing into lp:openlp
Review: Approve
880 + self.PreviewController = SlideController(self.ControlSplitter, False)
881 + self.LiveController = SlideController(self.ControlSplitter, True)
Just a note: I started naming things on forms before we had decided on a standard, so they're a little out of sync with the proper naming convention. I am cleaning them up in a cleanup branch, but just for reference, those two should probably be called "self.previewController"
982 + self.Toolbar.addToolbarButton("Move to top", ":/services/service_top.png",
983 + translate(u'ServiceManager', u'Move to start'), self.onServiceTop)
Not sure what the difference is between the first and second "move to top" strings is, but the first isn't u''
966 - self.Layout = QtGui.QVBoxLayout(self)
967 + self.Layout = QVBoxLayout(self)
Please rather use "from PyQt4 import QtCore, QtGui" as it pollutes the local namespace less. (I'm also cleaning this up in my local cleanup branch).
970 self.Toolbar = OpenLPToolbar(self)
1002 + self.ThemeComboBox = QComboBox(self.Toolbar)
1004 + self.ThemeWidget = QWidgetAction(self.Toolbar)
As noted before, my bad, these should be camelCase not PascalCase - also busy cleaning up in my branch.
Apart from those few minor things, everything looks fine. I'll still have to see these things in action to get a real understanding of what's going on though...
review approve
--
https://code.launchpad.net/~trb143/openlp/servicing/+merge/6161
Your team openlp.org Core is subscribed to branch lp:openlp.
Follow ups
References