openlp-core team mailing list archive

[rimach <crichter@xxxxxx>]

Hi Tim, thanks for your review! Here some comments from my side of view.

> 162 - Plugins should push stuff in and core save it.  Core should not go and
> grab it.  If they are media plugins then we need to new name a plugins will be
> confussing. It has for me!
I found no better solution till now :-(, because of the media stuff is used also if the media plugin is deactivated.
This stuff could also exist in other plugins, e.g. Alarm or later on a countdown plugin, ...
> 
> No check on inactive plugins.  This should be added and removed when plugins
> are activated / deactivated. Unless covered by above
Because of the the same code is used for background video, there should be no check for activated/deactivated plugins.
> Toolbar you do not need to next / prev slides so these nned to be removed.
What do you mean here?
> 1963 why do we need to add extra layer of object?
In my opionion its easier to handle (hide/show) if we have more controls later on (eg. DVD controls)
> 2582 why remove mediatab
I thought, I have to remove it because the mediatab now is handled seperately with getSettingsTab, isnt it?

> 766 is a python object so you should have get_display_CSS also 864 and 871
> 862 868 duplicate blank lines
> 920 MediaManager is python so need _'s in method names
> 949 # Signals  spaced needed.  In many places.
> 977 this is for plugins not core so it necessary change name.
> 1815 should be indented.
> 2275 Line length.  No lines over 80 characters
> 2422 blank line
I will change these issues.

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