openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing
53 self.trUtf8('No Servive item selected'),
54 self.trUtf8('You must select a existing Service Item to add to.'))

Firstly, "service item", not "Service item" nor "Service Item", and most definitely not "Servive" ;-) And secondly, line 53 up there is a Window title, so it should be "No Service Item Selected".


153 u'type':self.service_item_type,
154 u'audit':self.audit,
155 u'notes':self.notes,
157 u'preview':self.autoPreviewAllowed,
158 u'edit':self.edit_enabled,
159 u'maintain':self.maintain_allowed,
160 u'fromPlugin':self.fromPlugin

Please decide on a naming convention here... some of these are python_style and some of these are camelCase. Is this a Qt derived object, or a pure Python object? If the former, they all need to be camelCase, if the latter, they all need to be python_style.


177 def is_song(self):
179     return self.name.lower() == u'songs'
180	
181 def is_media(self):
182     return self.name.lower() == u'media'

I'm a bit worried about bits of code like this... if we're hard-coding stuff like this, and someone comes along with a new plugin, it doesn't conform to any of these, and then something fails or goes horribly wrong because we don't make provision for is.


286 from openlp.core.ui import DisplayHideMode

Maybe shorten it to just "HideMode"? That's quite a mouthful!


524 from openlp.core.ui import ServiceNoteForm,  ServiceItemEditForm
833 def blankScreen(self, blankType,  blanked=False):

Style police say, "no extra spaces!"



-- 
https://code.edge.launchpad.net/~trb143/openlp/newfeatures/+merge/22328
Your team OpenLP Core is subscribed to branch lp:openlp.