Merge lp://qastaging/~xavi-garcia-mena/unity-scope-mediascanner/music-youtube into lp://qastaging/unity-scope-mediascanner

Proposed by Xavi Garcia
Status: Merged
Approved by: Paweł Stołowski
Approved revision: 209
Merged at revision: 207
Proposed branch: lp://qastaging/~xavi-garcia-mena/unity-scope-mediascanner/music-youtube
Merge into: lp://qastaging/unity-scope-mediascanner
Diff against target: 363 lines (+222/-4)
6 files modified
src/musicaggregator/musicaggregatorquery.cpp (+64/-2)
src/musicaggregator/musicaggregatorquery.h (+3/-1)
src/musicaggregator/musicaggregatorscope.cpp (+3/-1)
src/musicaggregator/musicaggregatorscope.h (+2/-0)
tests/CMakeLists.txt (+16/-0)
tests/test-music-aggregator.cpp (+134/-0)
To merge this branch: bzr merge lp://qastaging/~xavi-garcia-mena/unity-scope-mediascanner/music-youtube
Reviewer Review Type Date Requested Status
Paweł Stołowski (community) Approve
Review via email: mp+240604@code.qastaging.launchpad.net

Commit message

Added the youtube scope in the music aggregator to search only for music entries.

Description of the change

Added the youtube scope in the music aggregator to search only for music entries.

To post a comment you must log in.
Revision history for this message
Paweł Stołowski (stolowski) wrote :

Looks good, thanks for adding the test for aggregator scope! Just two minor inline comments.

review: Needs Fixing
208. By Xavi Garcia

Removed department ID from the CannedQuery in the unit tests and added whitespaces between assignment.

Revision history for this message
Paweł Stołowski (stolowski) wrote :

Works fine with the corresponding branch of youtube scope, but there is an issue if you upgrade only the mediascanner scope, and leave yt scope outdated (which i believe is a valid scenario since we can't have version dependency?): in that case the aggregator displays non-music content as 'popular tracks on youtube'. Could we workaround that by filtering results out in the aggregator based on - say - category id in case yt scope doesn't know about the new aggregated:musicaggregator?

review: Needs Information
209. By Xavi Garcia

Added a check in the youtube music results to verify that the youtube scope understands the "aggregated:musicaggregator" department.

Revision history for this message
Paweł Stołowski (stolowski) wrote :

Yep, thanks, works fine. +1

review: Approve

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: