openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing
35	+ function getFlashMovieObject(movieName)
36	+ {
37	+ if (window.document[movieName])
38	+ {
39	+ return window.document[movieName];
40	+ }
41	+ if (navigator.appName.indexOf("Microsoft Internet")==-1)
42	+ {
43	+ if (document.embeds && document.embeds[movieName])
44	+ return document.embeds[movieName];
45	+ }
46	+ else // if (navigator.appName.indexOf("Microsoft Internet")!=-1)
47	+ {
48	+ return document.getElementById(movieName);
49	+ }
50	+ }

We're using WebKit. Figure out which one WebKit uses, and drop the rest please.

464	+class MediaStates(object):

Should be singular: MediaState.

466	+ An enumeratioin for possible States of the Media Player

"enumeration"

464	+class MediaStates(object):
465	+ """
466	+ An enumeratioin for possible States of the Media Player
467	+ (copied from Phonon::State
468	+ """
469	+ LoadingState = 0
470	+ StoppedState = 1
471	+ PlayingState = 2
472	+ PausedState = 4
473	+ OffState = 6

You don't need "State" at the end of each of those, because you'll always know you're dealing with a MediaState via the class, for example: MediaState.Loading or MediaState.Paused

565	+class MediaManager(object):
566	+ """
          ...

Your docstring needs to be formatted in reStructuredText.

(that's all for now, I'll look again another time and request more fixes.
-- 
https://code.launchpad.net/~crichter/openlp/media/+merge/58734
Your team OpenLP Core is subscribed to branch lp:openlp.