Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Review: Approve -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365811 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Review: Approve -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365811 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Review: Approve -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365811 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Review: Needs Fixing Please remove the media changes. -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365765 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Spoke to superfly last night Qt 5.12 is fine! -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365421 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
But if you do not have a defined set of mandatory packages you would have to insert suh a check for every non-core-python package that you import. What an ugly code would that make for! -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365421 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
No, the package can be removed even though you have setup.py which only you use. Packages can be removed by accident and then will go bang. Please do not change code which is fitting to a supported defensive pattern. -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365421 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Why do they break OpenLP? pymediainfo is marked in setup.py as mandatory dependency. Or do you mean the i18n string? -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365421 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Review: Needs Fixing
Please remove the video code changes as they break OpenLP and do not provide an
acceptable user experience.
Diff comments:
> === modified file 'openlp/core/ui/media/mediacontroller.py'
> --- openlp/core/ui/media/mediacontroller.py 2019-02-14 15:09:09 +
> +++ openlp/core/ui/media/mediacontroller.py 2019-04-02 17:19:11 +
> @@ -26,13 +26,8 @@
> import datetime
> import logging
>
> -try:
> -from pymediainfo import MediaInfo
> -pymediainfo_available = True
> -except ImportError:
> -pymediainfo_available = False
> -
> from subprocess import check_output
> +from pymediainfo import MediaInfo
This crashes the code with a stack trace and does not give an error message,
> from PyQt5 import QtCore, QtWidgets
>
> from openlp.core.state import State
> @@ -168,11 +163,11 @@
> self.setup()
> self.vlc_player = VlcPlayer(self)
> State().add_service("mediacontroller", 0)
> -if get_vlc() and pymediainfo_available:
> +if get_vlc():
> State().update_pre_conditions("mediacontroller", True)
> else:
> State().missing_text("mediacontroller",
> translate('OpenLP.SlideController',
> - "VLC or pymediainfo are missing, so you are
> unable to play any media"))
This does not give the correct message to the user about errors gracefully
providing errors and allowing them to continue degraded.
> + "VLC is missing, so you are unable to play
> any media"))
> self._generate_extensions_lists()
> return True
>
--
https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365421
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Instead of running python3 -m pytest in MP-04-macOS-Tests it could be changed to python3 setup.py test. This would take care of any dependencies and they would not have to be installed manually every time a dependency changes. -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365421 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
openlp/core/ui/style.py uses qdarkstyle. -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365379 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Review: Needs Fixing
See inline comment about wrong minimum version of Qt
Diff comments:
>
> === modified file 'scripts/check_dependencies.py'
> --- scripts/check_dependencies.py 2019-02-14 15:09:09 +
> +++ scripts/check_dependencies.py 2019-04-02 00:15:48 +
> @@ -40,8 +40,8 @@
>
> VERS = {
> 'Python': '3.6',
> -'PyQt5': '5.5',
> -'Qt5': '5.5',
> +'PyQt5': '5.12',
> +'Qt5': '5.12',
The minimum version should be the version supported by the latest Ubuntu LTS
(18.04) and the fedora equivalent. For Ububtu 18.04, this is Qt 5.10 !
> 'pymediainfo': '2.2',
> 'sqlalchemy': '0.5',
> 'enchant': '1.6'
--
https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365379
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~bastian-germann/openlp/depends into lp:openlp
Review: Needs Fixing As noted above there is some lint errors that must be fixed. I noticed you imported qdarkstyle, but it is not used anywhere? I assume it must be activated using a new setting? (I admit I have not run this branch) -- https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365379 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
