Merge lp://qastaging/~unity-team/unity-lens-music/improved-result-quality into lp://qastaging/unity-lens-music

Proposed by Mikkel Kamstrup Erlandsen
Status: Merged
Approved by: Michal Hruby
Approved revision: 89
Merged at revision: 86
Proposed branch: lp://qastaging/~unity-team/unity-lens-music/improved-result-quality
Merge into: lp://qastaging/unity-lens-music
Diff against target: 102 lines (+50/-25)
1 file modified
src/musicstore-collection.vala (+50/-25)
To merge this branch: bzr merge lp://qastaging/~unity-team/unity-lens-music/improved-result-quality
Reviewer Review Type Date Requested Status
Michal Hruby (community) Approve
Review via email: mp+113716@code.qastaging.launchpad.net

Commit message

Fix u1 search results to not seem too random. Lowers mem churn in musicstore scope quite a bit.

Description of the change

Fix u1 search results to not seem too random. Also fix potential crasher.

This was rooted in the fact that musicsearch.ubuntu.com returns a mix of artists, albums, and tracks in the results. We do not show artists, so if the results contains mainly that, then we're screwed (even though there could be many potential track/album matches outside the fetched results). Luckily newer versions of the webservice contains some parameters to tweak this behaviour. This commit makes us use that.

In addition to using the new musicsearch API I also lowered memory churn quite considerably by removing the need to create a lot of Track/Album objects and do excessive string copying.

To post a comment you must log in.
Revision history for this message
Michal Hruby (mhr3) wrote :

Nice, I find the presentation of results now more consistent with the rest of ulm - albums first, then individual tracks.

> Also fixed a bug where we'd pass possibly-null strings into a DeeModel. This could have been the root cause of bug #917974.

Actually null strings are handled fine by dee, I remember very well that the special casing for null strings to cause trouble when I was trying to optimize parts of dee, therefore the attached bug is a different issue and we can revert:

60 + model.append (uri != null ? uri : "",
61 + artwork_path != null ? artwork_path : "",
62 + PURCHASE_CATEGORY,
63 + mimetype != null ? mimetype : "",
64 + title != null ? title : "",
65 + artist != null ? artist : "",
66 + dnd_uri != null ? dnd_uri : "");

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Removed the null checks as requested. Also removed refs to crasher bug as it was unrelated after all.

Revision history for this message
Michal Hruby (mhr3) wrote :

Great. +1

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :
89. By Mikkel Kamstrup Erlandsen

Make updated musicstore-collection.vala work with Vala 0.14. We re-used variabel names in 2 for-loops inside the same method. Vala 0.14 no likey!

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Pushed r89 with a fix for Vala 0.14

Revision history for this message
Michal Hruby (mhr3) wrote :

Awesome!

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: