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

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

 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

review: Approve

« Back to merge proposal