openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

Tried running the code on linux and had a 10px border down the right and left side of the main display so I could see the slide below.

Need to update resources pyinstaller and create hook-openlp.plugins.media.core.ui.media.py file (check path)

ui/__init__  why add Display and Controller they are internal.

Display and Maindisplay have duplicate set up code for QTWebKit objects.

Alerts no longer get add over the top of videos.

media/__init__  imports should be at the top.

766 MediaAPI should be lowercase and any over calls.  This will stop case issues in the future.

922 self.APIs  can we be more explicit with the name.  What mediaPlayers for example.

925 currentDisplayMediaPlayer !

977 does media_api.py need to be in that directory?  

992 good example of why we need to rename.

999 is the comment correct.

1011 should there be a return here?  Then we could loose the confusing isAnyonePlaying.  If they are just leave.  The stop would be unconditional.

1113 - 1115 ?????????????

1434 should be a constant at top of class as duplicated.

1643 etc why split out the flash CSS javascript etc.

1871,1878  double blank line

2025 why Display

2404 etc name verticalLayout_X with a name

2620 Are all these translates going to be used?

2722 Commented out code

2794 "Chose Player" ??  need to check with Raoul

2811 Commented code

General comment use of API in variable names should be resisted.  Say what they are.

2992 how do we get here ?

MediaOpenForm  Cannot show this!

Hook-open.media,py is wrong very wrong!


































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