Re: Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
> On Aug. 14, 2012, 8:30 a.m., Bart Cerneels wrote: > > src/core-impl/collections/spotifycollection/SpotifySettings.cpp, line 217 > > <http://git.reviewboard.kde.org/r/105285/diff/3/?file=77322#file77322line217> > > > > You probably should do readAll() here. The data will be fully available > > after finished(). > > > > Unless there is some platform specific reason to do it like this? No specific reason but I was thinking the resolver binary might be very big, I once used a 100MB file to test downloading. Now it is replaced by readAll(). - Zhengliang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/#review17359 ------- On Aug. 13, 2012, 11:24 a.m., Zhengliang Feng wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105285/ > --- > > (Updated Aug. 13, 2012, 11:24 a.m.) > > > Review request for Amarok. > > > Description > --- > > Things I've done this week: > * Added a new playlist provider SpotifyPlaylistProvider > * Added playlist class SpotifyPlaylist > * Clear all extra whitespaces > * Figured out how Collection, QueryMaker and Playlist worked > > Things need to be done next week: > * Clean ScriptResolver's interfaces, move all query related interfaces into > Query class > * Replace Controller class with ScriptResolver > * Test SpotifyCollection > * Finish playlist and playlist provider > > > Diffs > - > > src/core-impl/collections/CMakeLists.txt c78b920 > src/core-impl/collections/spotifycollection/CMakeLists.txt PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfigWidget.ui > 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/SpotifyPlaylist.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylist.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.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/SpotifySettings.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifySettings.cpp > PRE-CREATION > > src/core-impl/collections/spotifycollection/amarok_collection-spotifycollection.desktop > 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/Query.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.h > PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.cpp > PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/105285/diff/ > > > Testing > --- > > No test done this week. > > > Thanks, > > Zhengliang Feng > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
> On Aug. 13, 2012, 4:43 p.m., Edward Hades Toroshchin wrote: > > src/core-impl/collections/spotifycollection/support/Controller.cpp, line 424 > > <http://git.reviewboard.kde.org/r/105285/diff/3/?file=77325#file77325line424> > > > > I think there should be a restart delay here. Imagine that the resolver > > quits immediately after startup. Hmm, you are right. I will use QTimer::singleShot() to do this. - Zhengliang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/#review17322 --- On Aug. 13, 2012, 11:24 a.m., Zhengliang Feng wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105285/ > --- > > (Updated Aug. 13, 2012, 11:24 a.m.) > > > Review request for Amarok. > > > Description > --- > > Things I've done this week: > * Added a new playlist provider SpotifyPlaylistProvider > * Added playlist class SpotifyPlaylist > * Clear all extra whitespaces > * Figured out how Collection, QueryMaker and Playlist worked > > Things need to be done next week: > * Clean ScriptResolver's interfaces, move all query related interfaces into > Query class > * Replace Controller class with ScriptResolver > * Test SpotifyCollection > * Finish playlist and playlist provider > > > Diffs > - > > src/core-impl/collections/CMakeLists.txt c78b920 > src/core-impl/collections/spotifycollection/CMakeLists.txt PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfigWidget.ui > 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/SpotifyPlaylist.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylist.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.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/SpotifySettings.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifySettings.cpp > PRE-CREATION > > src/core-impl/collections/spotifycollection/amarok_collection-spotifycollection.desktop > 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/Query.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.h > PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.cpp > PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/105285/diff/ > > > Testing > --- > > No test done this week. > > > Thanks, > > Zhengliang Feng > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/ --- (Updated Aug. 15, 2012, 6:47 a.m.) Review request for Amarok. Changes --- This diff solved all issues listed above. Some new changes: * Return immediately when already logged in as the same user * Clear collection when user changed * Try to login when Amarok is starting Description --- Things I've done this week: * Added a new playlist provider SpotifyPlaylistProvider * Added playlist class SpotifyPlaylist * Clear all extra whitespaces * Figured out how Collection, QueryMaker and Playlist worked Things need to be done next week: * Clean ScriptResolver's interfaces, move all query related interfaces into Query class * Replace Controller class with ScriptResolver * Test SpotifyCollection * Finish playlist and playlist provider Diffs (updated) - src/core-impl/collections/spotifycollection/SpotifyCollection.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyCollection.cpp PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyConfig.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyConfig.cpp 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/SpotifyPlaylist.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/SpotifySettings.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifySettings.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/Query.h PRE-CREATION src/core-impl/collections/spotifycollection/support/Query.cpp PRE-CREATION src/core-impl/collections/spotifycollection/support/TrackProxy.h PRE-CREATION src/core-impl/collections/spotifycollection/support/TrackProxy.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/105285/diff/ Testing --- No test done this week. Thanks, Zhengliang Feng ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
> On Aug. 14, 2012, 8:14 a.m., Bart Cerneels wrote: > > src/core-impl/collections/spotifycollection/SpotifyCollection.cpp, line 149 > > <http://git.reviewboard.kde.org/r/105285/diff/3/?file=77309#file77309line149> > > > > QAction is a QObject so could be parented to SpotifyCollection to be > > automatically deleted. Saves you some manual memory management. Fixed. > On Aug. 14, 2012, 8:14 a.m., Bart Cerneels wrote: > > src/core-impl/collections/spotifycollection/SpotifyConfig.h, line 45 > > <http://git.reviewboard.kde.org/r/105285/diff/3/?file=77310#file77310line45> > > > > API key is locked away in the resolver binary like intended. Remove > > these functions to avoid confusion. Fixed. > On Aug. 14, 2012, 8:14 a.m., Bart Cerneels wrote: > > src/core-impl/collections/spotifycollection/SpotifyConfig.cpp, line 72 > > <http://git.reviewboard.kde.org/r/105285/diff/3/?file=77311#file77311line72> > > > > Does it make sense to store this in the wallet? Doesn't look like > > sensitive info to me. Hmm, I forgot to remove it. Now fixed. - Zhengliang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/#review17355 --- On Aug. 13, 2012, 11:24 a.m., Zhengliang Feng wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105285/ > --- > > (Updated Aug. 13, 2012, 11:24 a.m.) > > > Review request for Amarok. > > > Description > --- > > Things I've done this week: > * Added a new playlist provider SpotifyPlaylistProvider > * Added playlist class SpotifyPlaylist > * Clear all extra whitespaces > * Figured out how Collection, QueryMaker and Playlist worked > > Things need to be done next week: > * Clean ScriptResolver's interfaces, move all query related interfaces into > Query class > * Replace Controller class with ScriptResolver > * Test SpotifyCollection > * Finish playlist and playlist provider > > > Diffs > - > > src/core-impl/collections/CMakeLists.txt c78b920 > src/core-impl/collections/spotifycollection/CMakeLists.txt PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfigWidget.ui > 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/SpotifyPlaylist.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylist.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.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/SpotifySettings.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifySettings.cpp > PRE-CREATION > > src/core-impl/collections/spotifycollection/amarok_collection-spotifycollection.desktop > 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/Query.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.h > PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.cpp > PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/105285/diff/ > > > Testing > --- > > No test done this week. > > > Thanks, > > Zhengliang Feng > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
> On Aug. 13, 2012, 4:43 p.m., Edward Hades Toroshchin wrote: > > Actually these are from PlaydarCollection, I will reformat all signal & slot names later. > On Aug. 13, 2012, 4:43 p.m., Edward Hades Toroshchin wrote: > > src/core-impl/collections/spotifycollection/support/Controller.cpp, line 266 > > <http://git.reviewboard.kde.org/r/105285/diff/3/?file=77325#file77325line266> > > > > I haven't found the place where query is deleted. The controller has a query cache, all query pointers are managed by KSharedPtr, a query emits queryDone() after finished or timed out, then the controller clear the reference count and removes the pointer from the cache, the query should be deleted then. I will add some test to double check this. > On Aug. 13, 2012, 4:43 p.m., Edward Hades Toroshchin wrote: > > src/core-impl/collections/spotifycollection/support/Controller.cpp, lines > > 462-486 > > <http://git.reviewboard.kde.org/r/105285/diff/3/?file=77325#file77325line462> > > > > Is this _really_ needed? Can spotify resolver be sometimes ".exe" and > > sometimes python script or ".doc"? :) Oh, this is because I borrowed ScriptResolver from Tomahawk, it supports scripts, I've removed this code. Thanks for pointing it out. :) - Zhengliang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/#review17322 --- On Aug. 13, 2012, 11:24 a.m., Zhengliang Feng wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105285/ > --- > > (Updated Aug. 13, 2012, 11:24 a.m.) > > > Review request for Amarok. > > > Description > --- > > Things I've done this week: > * Added a new playlist provider SpotifyPlaylistProvider > * Added playlist class SpotifyPlaylist > * Clear all extra whitespaces > * Figured out how Collection, QueryMaker and Playlist worked > > Things need to be done next week: > * Clean ScriptResolver's interfaces, move all query related interfaces into > Query class > * Replace Controller class with ScriptResolver > * Test SpotifyCollection > * Finish playlist and playlist provider > > > Diffs > - > > src/core-impl/collections/CMakeLists.txt c78b920 > src/core-impl/collections/spotifycollection/CMakeLists.txt PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyCollection.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfig.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyConfigWidget.ui > 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/SpotifyPlaylist.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylist.cpp > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.h > PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.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/SpotifySettings.h PRE-CREATION > src/core-impl/collections/spotifycollection/SpotifySettings.cpp > PRE-CREATION > > src/core-impl/collections/spotifycollection/amarok_collection-spotifycollection.desktop > 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/Query.cpp PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.h > PRE-CREATION > src/core-impl/collections/spotifycollection/support/TrackProxy.cpp > PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/105285/diff/ > > > Testing > --- > > No test done this week. > > > Thanks, > > Zhengliang Feng > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/ --- (Updated Aug. 13, 2012, 11:24 a.m.) Review request for Amarok. Changes --- Now the Spotify collection has a configuration dialog, all basic features are done except playlist synchronization. Updates: * Add a capability action for configuration dialog, and it works well. * Automatically download resolver program from remote server if it's not found in local system. * Remove KCModule code Todo: * Continue polishing Querymaker * Clean up unnecessary code and add more documentation * Finish playlist support Screenshots: [1] http://i.imgur.com/uh0KI.png [2] http://i.imgur.com/uLgmG.png [3] http://i.imgur.com/tFy3W.png Description --- Things I've done this week: * Added a new playlist provider SpotifyPlaylistProvider * Added playlist class SpotifyPlaylist * Clear all extra whitespaces * Figured out how Collection, QueryMaker and Playlist worked Things need to be done next week: * Clean ScriptResolver's interfaces, move all query related interfaces into Query class * Replace Controller class with ScriptResolver * Test SpotifyCollection * Finish playlist and playlist provider Diffs (updated) - src/core-impl/collections/CMakeLists.txt c78b920 src/core-impl/collections/spotifycollection/CMakeLists.txt PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyCollection.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyCollection.cpp PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyConfig.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyConfig.cpp PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyConfigWidget.ui 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/SpotifyPlaylist.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylist.cpp PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.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/SpotifySettings.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifySettings.cpp PRE-CREATION src/core-impl/collections/spotifycollection/amarok_collection-spotifycollection.desktop 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/Query.cpp PRE-CREATION src/core-impl/collections/spotifycollection/support/TrackProxy.h PRE-CREATION src/core-impl/collections/spotifycollection/support/TrackProxy.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/105285/diff/ Testing --- No test done this week. Thanks, Zhengliang Feng ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/ --- (Updated July 13, 2012, 6:04 p.m.) Review request for Amarok. Changes --- This is the latest diff between my work and master branch of Amarok. Currently this has the following problems: 1. Slot SpotifyQueryMaker::collectResults() will never be executed, I thought it is because the SpotifyQueryMaker is deleted before the results are returned. I added addToCollection() in Query::tracksAdded() to work around this. 2. The results are added to collection correctly, but sometimes the results don't show up in the collection browser. 3. The 'length' or 'duration' of a track is always 0, maybe it's a problem of Spotify resolver. Description --- Things I've done this week: * Added a new playlist provider SpotifyPlaylistProvider * Added playlist class SpotifyPlaylist * Clear all extra whitespaces * Figured out how Collection, QueryMaker and Playlist worked Things need to be done next week: * Clean ScriptResolver's interfaces, move all query related interfaces into Query class * Replace Controller class with ScriptResolver * Test SpotifyCollection * Finish playlist and playlist provider Diffs (updated) - src/core-impl/collections/CMakeLists.txt c78b920 src/core-impl/collections/spotifycollection/CMakeLists.txt PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyCollection.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyCollection.cpp 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/SpotifyPlaylist.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylist.cpp PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.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/Query.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/105285/diff/ Testing --- No test done this week. Thanks, Zhengliang Feng ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Review Request: GSoC report: Integrate Spotify into Amarok #4 (squashed commits, recent on top)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/ --- Review request for Amarok. Description --- Things I've done this week: * Added a new playlist provider SpotifyPlaylistProvider * Added playlist class SpotifyPlaylist * Clear all extra whitespaces * Figured out how Collection, QueryMaker and Playlist worked Things need to be done next week: * Clean ScriptResolver's interfaces, move all query related interfaces into Query class * Replace Controller class with ScriptResolver * Test SpotifyCollection * Finish playlist and playlist provider 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/SpotifyPlaylist.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylist.cpp PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.h PRE-CREATION src/core-impl/collections/spotifycollection/SpotifyPlaylistProvider.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/Query.cpp 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/105285/diff/ Testing --- No test done this week. Thanks, Zhengliang Feng ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #3 (squashed commits, recent on top)
> On June 11, 2012, 8:41 a.m., Bart Cerneels wrote: > > 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. Yes, some code like meta classe are just copy and paste. Some of them like Controller class might be deleted or replaced later, I was thinking about replacing it by ScriptResolver. QM is more complicated than I thought, it may take some time to fully understand how it works. Spotify can do specific search by using "artist:" or "album:" and using of logical operands like 'AND' or 'OR' is also possible, details are on https://developer.spotify.com/technologies/libspotify/guidelines-hardware/ in the 'Search' section. So it is easy to construct a search string for specific types. - Zhengliang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105201/#review14597 --- On June 11, 2012, 1:26 p.m., Zhengliang Feng wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105201/ > --- > > (Updated June 11, 2012, 1:26 p.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/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 > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #3 (squashed commits, recent on top)
> On June 11, 2012, 8:52 a.m., Leo Franchi wrote: > > One minor comment: > > > > Please don't remove copyright headers when copying files from another > > project--this is important for both legal and authorship (e.g. "who do I > > ask when this code doesn't work") reasons. The ScriptResolver.* files come > > from Tomahawk, and I'm totally happy with you re-using the code that we > > wrote. However, you need to retain the same copyright header that exists in > > the original source file, and additionally adding your own copyright if you > > modify the file. This is the original: > > > > https://github.com/tomahawk-player/tomahawk/blob/master/src/libtomahawk/resolvers/ScriptResolver.cpp > > > > This is also why the ScriptResolver class supports external .js and > > non-spotify resolvers. It'll work with any external process that implements > > the playdar + tomahawk extensions resolver API. > > > > thanks! Sorry about that, I forgot to add them when copying from the scratch repo. Now they are added in diff r2. - Zhengliang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105201/#review14599 --- On June 11, 2012, 1:26 p.m., Zhengliang Feng wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105201/ > --- > > (Updated June 11, 2012, 1:26 p.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/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 > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #3 (squashed commits, recent on top)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105201/ --- (Updated June 11, 2012, 1:26 p.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 (updated) - 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 ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Review Request: GSoC report: Integrate Spotify into Amarok (squashed commits, recent on top)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105201/ --- 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 ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: GSoC report: Integrate Spotify into Amarok #3 (squashed commits, recent on top)
--- 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. Changes --- Rename the title Summary (updated) - GSoC report: Integrate Spotify into Amarok #3 (squashed commits, recent on top) 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 ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
