----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105285/#review17355 -----------------------------------------------------------
The resolver failed to download for me. Got an QNetworkReply::UnknownNetworkError, but normally the binary would not have been saved correctly either. src/core-impl/collections/spotifycollection/SpotifyCollection.cpp <http://git.reviewboard.kde.org/r/105285/#comment13535> Use the enum value here. I assume this has to be ResolverNotFound src/core-impl/collections/spotifycollection/SpotifyCollection.cpp <http://git.reviewboard.kde.org/r/105285/#comment13536> QAction is a QObject so could be parented to SpotifyCollection to be automatically deleted. Saves you some manual memory management. src/core-impl/collections/spotifycollection/SpotifyConfig.h <http://git.reviewboard.kde.org/r/105285/#comment13537> API key is locked away in the resolver binary like intended. Remove these functions to avoid confusion. src/core-impl/collections/spotifycollection/SpotifyConfig.cpp <http://git.reviewboard.kde.org/r/105285/#comment13540> resolverPath does not have a default value and is never set except from kwallet/configfile. What is those are empty? It causes a failure to download in Settings. src/core-impl/collections/spotifycollection/SpotifyConfig.cpp <http://git.reviewboard.kde.org/r/105285/#comment13539> Does it make sense to store this in the wallet? Doesn't look like sensitive info to me. src/core-impl/collections/spotifycollection/SpotifySettings.cpp <http://git.reviewboard.kde.org/r/105285/#comment13538> You need to check QNetworkReply::error() == NoError before continuing. In case of download error the write will still be attempted. - Bart Cerneels 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
