Merge lp://staging/~stolowski/unity-lens-music/unity-lens-music.previews into lp://staging/unity-lens-music
Proposed by
Paweł Stołowski
Status: | Superseded |
---|---|
Proposed branch: | lp://staging/~stolowski/unity-lens-music/unity-lens-music.previews |
Merge into: | lp://staging/unity-lens-music |
Diff against target: |
1449 lines (+1074/-18) 17 files modified
configure.ac (+8/-0) data/Makefile.am (+4/-2) data/music-preview-player.service.in (+3/-0) po/POTFILES.in (+1/-0) po/POTFILES.skip (+1/-0) src/Makefile.am (+40/-3) src/banshee-collection.vala (+134/-7) src/banshee-scope.vala (+52/-0) src/music-preview-player.vala (+55/-0) src/musicstore-collection.vala (+74/-3) src/musicstore-scope.vala (+76/-0) src/player-service.vala (+77/-0) src/player.vala (+251/-0) src/preview-player-client.vala (+78/-0) src/rhythmbox-collection.vala (+63/-3) src/rhythmbox-scope.vala (+156/-0) src/track.vala (+1/-0) |
To merge this branch: | bzr merge lp://staging/~stolowski/unity-lens-music/unity-lens-music.previews |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michal Hruby (community) | Approve | ||
Unity Team | Pending | ||
Neil J. Patel | Pending | ||
Review via email:
|
This proposal has been superseded by a proposal from 2012-08-17.
Commit message
Implementation of previews.
Description of the change
Implementation of previews for Rhythmbox and Musicstore results; Banshee previews are limited (no playback).
To post a comment you must log in.
Nice work, works pretty well, but it's over 1k lines, so you know the drill ;)
478 + if (parser. load_from_ stream (file.read (null)))
Could you add a huge FIXME to make it async?
553 + preview_ uri_map. insert (uri.substring (7), details_uri);
And another comment explaining why 7 (u1ms://)?
745 + app.release();
Perhaps add a bool flag, so we don't call release multiple times from here?
1024 + public abstract async void pause () throws IOError;
1025 + public abstract async void pause_resume () throws IOError;
1026 + public abstract async void resume () throws IOError;
Do we really need 3 methods? Would one play_pause() suffice?
1259 + var show_folder_action = new Unity.PreviewAction ("show_in_folder", _("Show in Folder "), null);
1286 + var show_folder_action = new Unity.PreviewAction ("show_in_folder", _("Show in Folder "), null);
Extra space in the title.
911 + timer_id = GLib.Timeout. add_seconds (1, progress_ report_ cb);
Looks like this timer isn't removed when it needs to be, I saw it keep firing and spamming dbus long after the song finished.
There's also the issue that sorting of songs vs albums is broken now in trunk, although not fault of this branch, we should fix that asap.