← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~trb143/openlp/servicing2 into lp:openlp

 

Review: Approve
Some nitpicks and queries:
8	+ #replace the quotes with quotes
9	+ line, replace("''", "'")
Do you need a "u" in front of the strings?  Also, I find it easier to see what's going on in strings that are pure quote marks to actually escape the quotes (even though it's unnecessary).  
Also, should it be line=line.replace(...)?

808	+ self.SlidePreview.setFixedSize(QtCore.QSize(250, 210))
Should the slide preview be of fixed size?  Does this cause it to be too big on lo-res screen?

873	+class MasterToolbar(QtCore.QObject):
874	+ """
875	+ Class from which all tollbars should extend
typo - tollbars -> toolbars.

1096	+ #check to see file is in route directory
*root* directory?

The timer for the image lists:  should the timeout be configurable as part of the OOS (different per service item), rather than in the plugin?  Or is the settings tab just for a default?


-- 
https://code.launchpad.net/~trb143/openlp/servicing2/+merge/8210
Your team openlp.org Core is subscribed to branch lp:openlp.



Follow ups

References