Review Request: GSoC report: Integrate Spotify into Amarok #3 (squashed commits, recent on top)

Bart Cerneels bart.cerneels at kde.org
Mon Jun 11 08:40:57 UTC 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105201/#review14597
-----------------------------------------------------------


I'm assuming this is mostly code copied and edited (but not completely) from the playdar work done by Andy for GSoC 2010.
As far as I understand it the protocol used by the Tomahawk resolver is still the same (playdar API), so that should work. There are a few things you should look at before going on with this design though.
First do some namespace cleanup. ScriptResolver for instance is a confusing name. Here you won't be dealing with a resolver script but the spotify-resolver application. ResolverProcessInterface looks a little long, but does cover it's function pretty well. Up to you to figure out a good name.

I've not gone through the architecture of the playdar QueryMaker. It probably works, but I wonder if it has to be so complicated. QM is not a simple API to begin with though. Spotify does not support search strings for specific types (artist, album, genre, etc) AFAIU, so a mapping might be more straight forward. If this code is working keep it for now, as long as no performance issues pop up.


src/core-impl/collections/spotifycollection/SpotifyMeta.h
<http://git.reviewboard.kde.org/r/105201/#comment11543>

    This whitespace at end of line can be prevented by using the option "Clean whitespaces" in Text Editor > Behavior of the QtCreator settings.


- Bart Cerneels


On June 10, 2012, 7:10 a.m., Zhengliang Feng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105201/
> -----------------------------------------------------------
> 
> (Updated June 10, 2012, 7:10 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Add Spotify collection code
> 
> Currently implemented SpotifyCollection, SpotifyQueryMaker and
> SpotifyMeta. The ScriptResolver is the class handles communcation with
> standalone Spotify resolver, the code is mainly from original
> ScriptResolver, but added more functions to handle messages separately.
> 
> The controller class is used to start a ScriptResolver in a separate
> thread and handles queries.
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/CMakeLists.txt c78b9202ece71b51189c4e47d85acfa4a74ef8d6 
>   src/core-impl/collections/spotifycollection/CMakeLists.txt PRE-CREATION 
>   src/core-impl/collections/spotifycollection/SpotifyCollection.h PRE-CREATION 
>   src/core-impl/collections/spotifycollection/SpotifyMeta.h PRE-CREATION 
>   src/core-impl/collections/spotifycollection/SpotifyMeta.cpp PRE-CREATION 
>   src/core-impl/collections/spotifycollection/SpotifyQueryMaker.h PRE-CREATION 
>   src/core-impl/collections/spotifycollection/SpotifyQueryMaker.cpp PRE-CREATION 
>   src/core-impl/collections/spotifycollection/support/Controller.h PRE-CREATION 
>   src/core-impl/collections/spotifycollection/support/Controller.cpp PRE-CREATION 
>   src/core-impl/collections/spotifycollection/support/QMFunctionTypes.h PRE-CREATION 
>   src/core-impl/collections/spotifycollection/support/Query.h PRE-CREATION 
>   src/core-impl/collections/spotifycollection/support/ScriptResolver.h PRE-CREATION 
>   src/core-impl/collections/spotifycollection/support/ScriptResolver.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105201/diff/
> 
> 
> Testing
> -------
> 
> Communication between ScriptResolver and Spotify resolver( from Tomahawk resolver repo https://github.com/ofan/tomahawk-resolvers ).
> Logging into Spotify using a username and password.
> 
> 
> Thanks,
> 
> Zhengliang Feng
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120611/40c7df36/attachment.html>


More information about the Amarok-devel mailing list