Review: Needs Fixing
23 from renderer import Renderer
24 from openlp.core.lib import ThemeLevel
25 (deleted)
26 log = logging.getLogger(__name__)

Please don't delete that open line.


94 def updateCapability(self, capability, State=False):
95     self.capability_state[capability] = State

Please use a lowercase "s" for "state".


94 def updateCapability(self, capability, State=False):

97 def getCapability(self, capability):

ServiceItem is not a Qt object, please rename these methods.


68 capabilities = [
69     u'allows_preview',
70     u'allows_edit',
71     u'allows_maintain',
72     u'requires_media'
73 ]

Rather make this an "enumeration". It is more accurate and less prone to typing 
mistakes.

class ItemCapabilities(object):
    AllowsPreview = 1
    AllowsEdit = 2
    AllowsMaintain = 3
    RequiresMedia = 4

This also makes your life easier, because you don't need to maintain a dict, 
you can maintain a simple list, and if that capability is not in the list, that 
service item doesn't have that capability - once again lessening the 
probability of bugs being introduced.

A simple test would be:

  if ItemCapabilities.AllowsEdit in self.capabilities:
      # do something


+ self.config.set_config(u'monitor display', self.MonitorDisplay)

What is "monitor display" and self.MonitorDisplay? If it's a boolean, rather 
call it "display monitor" and self.DisplayMonitor, as these make more sense. 
"monitor display" sounds like it could be which monitor to put the display 
screen on.


Please also just watch your indentation... line wraps should be indented 4 
spaces.
-- 
https://code.launchpad.net/~trb143/openlp/bugs1/+merge/22722
Your team OpenLP 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

Reply via email to