← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~mjthompson/openlp/media_plugin into lp:openlp

 

Review: Needs Fixing
Please don't use this, it throws Eric4 out (and is incorrect Python):

  assert 0, 'This fn needs to be defined by the plugin'

Rather use:

  raise NotImplementedError(u'This function needs to be defined by the plugin')


Rather return None, and do a check on the other side... "image is not None":

264	+        except:
265	+            log.info("Can't generate video preview for some reason");
266	+            import sys
267	+            print sys.exc_info()
268	+            return QtGui.QImage()


This should be in a docstring:
43	     # self.ListViewWithDnD_class - there is a base list class with DnD assigned to it (openlp.core.lib.BaseListWithDnD())
44	     # each plugin needs to inherit a class from this and pass that *class* (not an instance) to here
45	     # via the ListViewWithDnD_class member
46	+    # self.ServiceItemIconName - string referring to an icon file or a resource icon
47	+    # self.PreviewFunction - a function which returns a QImage to represent the item (a preview usually) - no scaling required - that's done later
48	+    #                        If this fn is not defined, a default will be used (treat the filename as an image)
49	+
50	     # The assumption is that given that at least two plugins are of the form
51	     # "text with an icon" then all this will help
52	     # even for plugins of another sort, the setup of the right-click menu, common toolbar


-- 
https://code.launchpad.net/~mjthompson/openlp/media_plugin/+merge/8066
Your team openlp.org Core is subscribed to branch lp:openlp.



References