[Merge] lp:~chunsang/media-hub/wip_media-hub-interface into lp:media-hub
Jim Hodapp
jim.hodapp at canonical.com
Tue Mar 21 17:55:16 UTC 2017
Review: Needs Fixing code
A few things to fix. See inline below.
Diff comments:
> === added directory 'overlay'
> === added directory 'overlay/bin'
> === added file 'overlay/bin/media-hub-service.wrapper'
> --- overlay/bin/media-hub-service.wrapper 1970-01-01 00:00:00 +0000
> +++ overlay/bin/media-hub-service.wrapper 2017-03-19 07:35:27 +0000
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +#
> +# Copyright (C) 2017 Canonical Ltd
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License version 3 as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +if [ -e /var/run/pulse ]; then
> + # PA as a snap
> + export PULSE_RUNTIME_PATH=/var/run/pulse
> + export PULSE_SYSTEM=1
> +else
> + # classic
> + export PULSE_RUNTIME_PATH=/run/user/$(id -u)/pulse
> +fi
> +
> +exec $SNAP/bin/desktop-launch "$@"
Is this really the right thing to launch media-hub with? Is this giving you something in the environment that simply doing: 'exec "media-hub-server" "$@"' can't accomplish?
>
> === added directory 'overlay/ubuntu-app-platform'
> === modified file 'snapcraft.yaml'
> --- snapcraft.yaml 2017-02-17 15:56:44 +0000
> +++ snapcraft.yaml 2017-03-19 07:35:27 +0000
> @@ -75,6 +105,12 @@
> - -usr/lib/*.la
> - -usr/lib/*/*.la
> - -usr/lib/*/*.o
> + after:
> + - desktop-ubuntu-app-platform
>
> - snap:
> + prime:
> - $unwanted
> +
> + media-hub-overlay:
Can you add a comment as to what this section's purpose is just so it's clear to the next person who looks at this snapcraft.yaml file?
> + source: overlay/
> + plugin: dump
>
> === modified file 'src/core/media/apparmor/ubuntu.cpp'
> --- src/core/media/apparmor/ubuntu.cpp 2016-04-06 15:42:15 +0000
> +++ src/core/media/apparmor/ubuntu.cpp 2017-03-19 07:35:27 +0000
> @@ -85,7 +89,7 @@
> static const std::regex full_re{"(.*)_(.*)_(.*)"};
> static const std::regex trust_store_re{"(.*)-(.*)"};
>
> - if ((s == "messaging-app" or s == unity_name)
> + if ((s == "messaging-app" or s == unity_name or s == unity8_snap_name or s == mediaplayer_snap_name)
You may want to check with ahayzen quickly and add the snap name for music-app as well while we're modifying this section. Should be a very easy change.
> and std::regex_match(s, out, trust_store_re))
> {
> pkg_name = s;
--
https://code.launchpad.net/~chunsang/media-hub/wip_media-hub-interface/+merge/319896
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list