openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

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.