openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Review: Needs Fixing



Diff comments:

> 
> === modified file 'openlp/core/ui/projector/manager.py'
> --- openlp/core/ui/projector/manager.py	2017-05-27 18:21:24 +0000
> +++ openlp/core/ui/projector/manager.py	2017-06-09 14:14:27 +0000
> @@ -278,6 +279,10 @@
>      """
>      Manage the projectors.
>      """
> +    projector_list = []
> +    pjlink_udp = PJLinkUDP()
> +    pjlink_udp.projector_list = projector_list
> +

This looks like you're acting on the class? Why are you doing this? Why not in __init__() ?

>      def __init__(self, parent=None, projectordb=None):
>          """
>          Basic initialization.
> 
> === modified file 'openlp/plugins/songs/lib/upgrade.py'
> --- openlp/plugins/songs/lib/upgrade.py	2017-03-23 05:31:51 +0000
> +++ openlp/plugins/songs/lib/upgrade.py	2017-06-09 14:14:27 +0000
> @@ -137,7 +138,17 @@
>          op.execute('INSERT INTO authors_songs_tmp SELECT author_id, song_id, "" FROM authors_songs')
>          op.drop_table('authors_songs')
>          op.rename_table('authors_songs_tmp', 'authors_songs')
> +
> +
> +def upgrade_7(session, metadata):

Please stick this back at 6. Our current production version is 5, and we haven't actually released this code yet, so the devs can just deal with the problems.

> +    """
> +    Version 7 upgrade
> +
> +    Corrects table error in upgrade 5
> +    """
>      # Move upgrade 5 here to correct it
> +    op = get_upgrade_op(session)
> +    metadata.reflect()
>      if 'songs_songbooks' not in [t.name for t in metadata.tables.values()]:
>          # Create the mapping table (songs <-> songbooks)
>          op.create_table(


-- 
https://code.launchpad.net/~alisonken1/openlp/pjlink2-e/+merge/325388
Your team OpenLP Core is subscribed to branch lp:openlp.