Review: Approve rereview
A few small issues, nothing major though. You can just change your local branch
so that it comes in the next merge.
Remember your spaces:
log.debug(u'plugin %s registered with EventManager'%plugin)
should be
log.debug(u'plugin %s registered with EventManager' % plugin)
Remember your unicode, even in the "translate" function:
AlertForm.setWindowTitle(translate("AlertForm", u'Alert Message'))
should be
AlertForm.setWindowTitle(translate(u'AlertForm', u'Alert Message'))
As we finish up, we should work through all of our forms, and replace the Qt
translate function with our core translate function. It is simply a wrapper,
but this gives us more flexibility.
self.FileNewItem.setText(QtGui.QApplication.translate("main_window",
"&New", None, QtGui.QApplication.UnicodeUTF8))
will change to
self.FileNewItem.setText(translate(u'MainWindow', u'&New'))
In AlertForm:
QtCore.QObject.connect(self.CancelButton, QtCore.SIGNAL("clicked()"),
AlertForm.close)
QtCore.QObject.connect(self.DisplayButton, QtCore.SIGNAL("clicked()"),
self.onDisplayClicked)
QDialog has "accepted" and "rejected" slots, you should connect the cancel
button to "rejected" rather than "close" - this allows the dialog to do any
cleanup and other stuff necessary, and returns the "rejected" status to exec_()
Possibly also look into calling the "accepted" slot in onDisplayClicked()
--
https://code.launchpad.net/~trb143/openlp/trb143/+merge/4948
Your team openlp.org Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
