← Back to team overview

openlp-core team mailing list archive

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