← Back to team overview

openlp-core team mailing list archive

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

 

Review: Needs Information
I agree with Martin's comments, things seem rather confusing. The toolbar class shouldn't contain the methods to add things to the list view, that should happen in the SlideController class. Also, the onSlideXXX methods should be in the SlideController class, not the toolbar class (I think you can map multiple signals to 1 method/slot).


In addition to this, docstrings are written in reStructuredText, so just be aware that in these lists of parameters you need a line between them, both above and below:

16	There are three types of render
17	``text``
18	    Where the renderManager is used to build the display frames

16	There are three types of render
17	
18	``text``
19	    Where the renderManager is used to build the display frames


What's with the small "i"?

283	class impressToolbar(MasterToolbar):


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



References