openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing
51	+ if (navigator.appName.indexOf("Microsoft Internet")==-1)
52	+ {
53	+ if (document.embeds && document.embeds[movieName])
54	+ return document.embeds[movieName];
55	+ }

Please remove all traces of stuff that references Internet Explorer. We are using WebKit, this code doesn't need to be here and only serves to add bloat where it is not needed.

616	+ def sendToPlugins(self, v1=None, v2=None, v3=None, v4=None, v5=None):

What is "v1", "v2", etc? They don't make sense. If you need to add multiple arguments and you don't know how many, either use *args or **kwargs.

767	+class MediaAPIs(object):

Enumeration objects should be singular ("MediaAPI"). You can also combine the enumeration and the actual MediaAPI object (Qt does this).

This is all I have to say so far, without further testing. I'll have a look again at home.


-- 
https://code.launchpad.net/~crichter/openlp/media_rewrite/+merge/63638
Your team OpenLP Core is subscribed to branch lp:openlp.