Code review comment for lp:~pete-woods/unity-scope-soundcloud/native-rewrite

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

Tested the previews in real dash, and they work fine, it's just:

897 + addPreviewData(previewer, preview, "artist", _("Artist"));
898 + addPreviewData(previewer, preview, "album", _("Album"));
899 + addPreviewData(previewer, preview, "genre", _("Genre"));
900 + addPreviewData(previewer, preview, "label", _("Label"));
901 + addPreviewData(previewer, preview, "license", _("License"));
902 + addPreviewData(previewer, preview, "creation_date", _("Creation Date"));

The music preview renderer doesn't handle this correctly, pls remove.

921 + g_variant_get_string(stream, NULL), 1,

The stream URI isn't correct, it's missing the API key, and therefore it can't be played.

Perhaps the best solution would be to use GenericPreview if the stream URI isn't defined (plus keep all the metadata) and if it is present, use MusicPreview.

I also noticed a few:
QSocketNotifier: Can only be used with threads started with QThread

Is that something to be worried about? FWIW if you provide preview_async_func, you can spawn a QThread there and invoke the callback once the thread finishes.

review: Needs Fixing

« Back to merge proposal