Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-s into lp:openlp

2018-06-28 Thread Tomas Groth 
Review: Approve

Looks good to me
-- 
https://code.launchpad.net/~alisonken1/openlp/pjlink2-s/+merge/348709
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:~alisonken1/openlp/pjlink2-s into lp:openlp

2018-06-28 Thread Tim Bentley 
Review: Approve


-- 
https://code.launchpad.net/~alisonken1/openlp/pjlink2-s/+merge/348709
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:~alisonken1/openlp/pjlink2-s into lp:openlp

2018-06-28 Thread Tim Bentley 
Review: Approve


-- 
https://code.launchpad.net/~alisonken1/openlp/pjlink2-s/+merge/348698
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:~alisonken1/openlp/pjlink2-s into lp:openlp

2018-06-28 Thread Tim Bentley 
Review: Needs Fixing

The fix for version is not correct see inline notes.


Diff comments:

> 
> === modified file 'openlp/core/version.py'
> --- openlp/core/version.py2018-03-29 15:54:55 +
> +++ openlp/core/version.py2018-06-28 15:48:46 +
> @@ -89,6 +89,10 @@
>  while retries < 3:
>  try:
>  response = requests.get(download_url, headers=headers)
> +if response.status_code != 200:

No this is not correct 
if status == 200 then set remote_version otherwise leave as null.
See how I have fixed it in the fonts branch,

> +log.warn('Server returned status code {code}: '
> + 'Version update check not 
> available.'.format(code=response.status_code))
> +break
>  remote_version = response.text.strip()
>  log.debug('New version found: %s', remote_version)
>  break


-- 
https://code.launchpad.net/~alisonken1/openlp/pjlink2-s/+merge/348698
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