[Merge] lp:~renatofilho/mediaplayer-app/shared-snappy into lp:mediaplayer-app
Jim Hodapp
jim.hodapp at canonical.com
Fri Nov 18 14:57:03 UTC 2016
Review: Needs Fixing code
A couple of issues below.
Diff comments:
> === modified file 'config.h.in'
> --- config.h.in 2014-10-01 21:29:22 +0000
> +++ config.h.in 2016-11-17 23:15:21 +0000
> @@ -29,7 +29,11 @@
> }
>
> inline QString mediaPlayerDirectory() {
> - if (isRunningInstalled()) {
> + static const QByteArray SNAP_PATH("SNAP");
> +
> + if (qEnvironmentVariableIsSet(SNAP_PATH)) {
You've tested this both on the desktop and on a phone?
> + return QString("%1/@MEDIAPLAYER_DIR@").arg(QString(qgetenv(SNAP_PATH)));
> + } else if (isRunningInstalled()) {
> return QString("@MEDIAPLAYER_DIR@");
> } else {
> return QString("@mediaplayer_src_SOURCE_DIR@");
>
> === added directory 'snap'
> === added directory 'snap/ubuntu-app-platform'
> === modified file 'snapcraft.yaml'
> --- snapcraft.yaml 2016-09-08 20:56:15 +0000
> +++ snapcraft.yaml 2016-11-17 23:15:21 +0000
> @@ -32,9 +39,7 @@
> - -DCMAKE_INSTALL_PREFIX:PATH=/usr
> - -DCMAKE_LIBRARY_PATH=/usr/lib
>
> - stage-packages:
> - - qtubuntu-media
> - - qtdeclarative5-ubuntu-content1
> + stage-packages: []
Any reason to keep this around if empty?
>
> filesets:
> unwanted:
--
https://code.launchpad.net/~renatofilho/mediaplayer-app/shared-snappy/+merge/311212
Your team Ubuntu Phablet Team is subscribed to branch lp:mediaplayer-app.
More information about the Ubuntu-reviews
mailing list