Ubuntu Music App

Merge lp:~ken-vandine/music-app/content-hub-desktop into lp:music-app

  1. content-hub-desktop
  2. Merge into trunk
Proposed by Ken VanDine
Status: Merged
Approved by: Victor Thompson
Approved revision: 955
Merged at revision: 953
Proposed branch: lp:~ken-vandine/music-app/content-hub-desktop
Merge into: lp:music-app
Prerequisite: lp:~ahayzen/music-app/release-2.2ubuntu2
Diff against target: 57 lines (+10/-4)
2 files modified
CMakeLists.txt (+6/-4)
debian/changelog (+4/-0)
To merge this branch: bzr merge lp:~ken-vandine/music-app/content-hub-desktop
Reviewer Review Type Date Requested Status
Jenkins Bot continuous-integration Approve
Andrew Hayzen Approve
Victor Thompson Approve
Review via email: mp+279515@code.launchpad.net

This proposal supersedes a proposal from 2015-12-02.

Commit message

Install the content-hub peer registration json file properly when installing from a deb. Renamed .desktop file when installed from deb.

Description of the change

Install the content-hub peer registration json file properly when installing from a deb. Renamed .desktop file when installed from deb.

To post a comment you must log in.
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:952
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/14/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/213/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/14/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Andrew Hayzen (ahayzen) wrote : Posted in a previous version of this proposal

Thanks for this!

One inline comment so far, also I am about to bump the version in the changelog (as we just had a release) once this is done could you pull and add an entry for yourself in there :-)

review: Needs Fixing
Revision history for this message
Andrew Hayzen (ahayzen) wrote : Posted in a previous version of this proposal

Also this fails to build with click [0] [1]

And for reference this is the MP with the version bump and release in the changelog etc [2]

0 - http://pastebin.ubuntu.com/13645357/
1 - https://core-apps-jenkins.ubuntu.com/job/music-app-ci/14/console
2 - https://code.launchpad.net/~ahayzen/music-app/release-2.2ubuntu2/+merge/279444

review: Needs Fixing
Revision history for this message
Ken VanDine (ken-vandine) : Posted in a previous version of this proposal
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:953
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/16/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/219/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/16/rebuild

review: Approve (continuous-integration)
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:955
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/17/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/220/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/17/rebuild

review: Approve (continuous-integration)
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:955
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/18/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/223/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/music-app-ci/18/rebuild

review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

Ken, is there an easy way to test this? I'm building the deb (dpkg-buildpackage -uc -us), installing it, then installing ubuntu-filemanager-app via my ppa [1]. The filemanager doesn't see the music app (or any app on my desktop) as a content importer.

1 - https://launchpad.net/~vthompson/+archive/ubuntu/ppa

review: Needs Information
Revision history for this message
Ken VanDine (ken-vandine) wrote :

You have to run the content-hub hook manually, /usr/lib/x86_64-linux-gnu/content-hub/content-hub-peer-hook

We don't have a good solution for that yet, since it has to be run by the user not root we can't do it with package install. However, if you restart your session, i think it'll run via upstart.

Revision history for this message
Victor Thompson (vthompson) wrote :

There don't appear to be any regressions on the device. However, I still can't get this to work on the desktop (unity7). In order to have the content hub hook on my system, I had to manually install the content-hub package/service. Should this be added to the app's debian control file?

review: Needs Information
Revision history for this message
Ken VanDine (ken-vandine) wrote :

> There don't appear to be any regressions on the device. However, I still can't
> get this to work on the desktop (unity7). In order to have the content hub
> hook on my system, I had to manually install the content-hub package/service.
> Should this be added to the app's debian control file?

I don't think it should be a hard depends, at least not yet. If the depends isn't there, it just won't use that feature. But I'm fine adding the depends if you would prefer.

Revision history for this message
Victor Thompson (vthompson) wrote :

Thanks Ken. lgtm!

review: Approve
Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Awesome, click builds and my points have been addressed. LGTM, thanks Ken :-)

review: Approve
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2015-08-08 18:32:22 +0000
3+++ CMakeLists.txt 2015-12-03 21:46:25 +0000
4@@ -38,6 +38,8 @@
5 configure_file(manifest.json.in ${CMAKE_CURRENT_BINARY_DIR}/manifest.json)
6 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/manifest.json apparmor.json
7 music-app-content.json DESTINATION ${CMAKE_INSTALL_PREFIX})
8+ install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${DESKTOP_FILE}
9+ DESTINATION ${DESKTOP_DIR})
10 else(CLICK_MODE)
11 set(DATA_DIR ${CMAKE_INSTALL_DATADIR}/${APP_HARDCODE})
12 set(EXEC ${APP_HARDCODE})
13@@ -48,6 +50,9 @@
14 DESTINATION ${CMAKE_INSTALL_BINDIR})
15 set(DESKTOP_DIR ${CMAKE_INSTALL_DATADIR}/applications)
16 set(URLS_DIR ${CMAKE_INSTALL_DATADIR}/url-dispatcher/urls)
17+ install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${DESKTOP_FILE}
18+ DESTINATION ${DESKTOP_DIR} RENAME music-app.desktop)
19+ install(FILES music-app-content.json DESTINATION ${CMAKE_INSTALL_DATADIR}/content-hub/peers/ RENAME music-app)
20 endif(CLICK_MODE)
21
22 file(GLOB_RECURSE I18N_SRC_FILES
23@@ -60,7 +65,7 @@
24
25 file(GLOB SRC_FILES
26 RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
27- *.qml *.js *.png *.js *.json)
28+ *.qml *.js *.png *.js)
29 install(DIRECTORY app DESTINATION ${DATA_DIR})
30 install(FILES ${SRC_FILES} ${ICON_FILE} DESTINATION ${DATA_DIR})
31
32@@ -71,9 +76,6 @@
33 COMMAND LC_ALL=C ${INTLTOOL_MERGE} -d -u ${CMAKE_SOURCE_DIR}/po ${DESKTOP_FILE}.in ${DESKTOP_FILE}
34 )
35
36-install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${DESKTOP_FILE}
37- DESTINATION ${DESKTOP_DIR})
38-
39 install(FILES ${URLS_FILE} DESTINATION ${URLS_DIR})
40
41 # Tests
42
43=== modified file 'debian/changelog'
44--- debian/changelog 2015-12-03 21:46:25 +0000
45+++ debian/changelog 2015-12-03 21:46:25 +0000
46@@ -1,7 +1,11 @@
47 music-app (2.3) UNRELEASED; urgency=medium
48
49+ [ Andrew Hayzen ]
50 * Release 2.2ubuntu2 and start on 2.3
51
52+ [ Ken VanDine ]
53+ * Install the content-hub json file in the correct place for peer registry
54+
55 -- Andrew Hayzen <ahayzen@gmail.com> Thu, 03 Dec 2015 14:11:35 +0000
56
57 music-app (2.2ubuntu2) vivid; urgency=medium

Subscribers

People subscribed via source and target branches