← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~smpettit/openlp/data-path into lp:openlp

 

Review: Needs Fixing

36	+ self.dataDirectoryLayout =QtGui.QFormLayout(self.dataDirectoryGroupBox)
38	+ self.dataDirectoryCurrentLabel= QtGui.QLabel(self.dataDirectoryGroupBox)
41	+ self.dataDirectoryLabel= QtGui.QLabel(self.dataDirectoryGroupBox)
43	+ self.dataDirectoryNewLabel= QtGui.QLabel(self.dataDirectoryGroupBox)

Spaces around the equals signs please.


184	+ 'OpenLP data directory was not found \n\n %s \n\n'

You don't need those spaces on either side of the \n\n's.


197	+ log.exception(u'User requested termination')

log.exception() is only for when an exception occurs. If there is no error, and it's simply a logging message, use log.info().


218	+ old_root_path = unicode(str(self.dataDirectoryLabel.text()))

Why are you casting to a string, and then to unicode? You're essentially doing the same thing twice, except that using str() looses non-ASCII characters and will make you extremely unpopular amongst our non-English users ;-)


237	+ 'data directory to:\n\n %s \n\n'

No spaces are necessary.


(there's more stuff that needs to be looked at, but you can look at fixing some of this so long)
-- 
https://code.launchpad.net/~smpettit/openlp/data-path/+merge/104657
Your team OpenLP Core is subscribed to branch lp:openlp.


References