Unity Media Scanner Scope

Merge lp://qastaging/~jamesh/unity-scope-mediascanner/bug-1367334 into lp://qastaging/unity-scope-mediascanner

  1. bug-1367334
  2. Merge into trunk
Proposed by James Henstridge
Status: Merged
Approved by: Jussi Pakkanen
Approved revision: 180
Merged at revision: 175
Proposed branch: lp://qastaging/~jamesh/unity-scope-mediascanner/bug-1367334
Merge into: lp://qastaging/unity-scope-mediascanner
Diff against target: 309 lines (+181/-21)
3 files modified
src/music-scope.cpp (+8/-4)
tests/test-music-scope.cpp (+132/-11)
tests/test-video-scope.cpp (+41/-6)
To merge this branch: bzr merge lp://qastaging/~jamesh/unity-scope-mediascanner/bug-1367334
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Jussi Pakkanen (community) Needs Fixing
Review via email: mp+234055@code.qastaging.launchpad.net

Commit message

Fix in-preview song playback in previews for individual songs. Also, flesh out the tests for music and video previews.

Description of the change

When we switched over to using music:/// URIs to launch the music app from previews, the change was made in more places than wanted. In particular, it affected the URIs we used for the track list in the previews which broke in-preview playback.

This branch changes things so we only produce music:/// URIs in the action button itself. It also adds tests for the previews of both the local music and video scopes (which wasn't possible when I was first adding test coverage).

To post a comment you must log in.
lp://qastaging/~jamesh/unity-scope-mediascanner/bug-1367334 updated
179. By James Henstridge

Remove unneeded comma.

Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

Looking good.

There are only two minor points in test-music-scope.cpp that are not related to this MR but let's fix them at the same time. On line 34 NULL is used instead of nullptr and MusicScopeTest has virtual functions but not a virtual destructor.

review: Needs Fixing
lp://qastaging/~jamesh/unity-scope-mediascanner/bug-1367334 updated
180. By James Henstridge

Use nullptr rather than NULL.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:179
http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-ci/144/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-amd64-ci/46
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-armhf-ci/46
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-i386-ci/46

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-mediascanner-ci/144/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:180
http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-ci/145/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-amd64-ci/47
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-armhf-ci/47
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-i386-ci/47

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-mediascanner-ci/145/rebuild

review: Approve (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

I've made the s/NULL/nullptr/ changes.

As discussed on IRC, the virtual destructor issue seems to be a false positive from the lint tool Jussi used.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: