----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110082/#review31357 -----------------------------------------------------------
src/browsers/playlistbrowser/PlaylistBrowserModel.cpp <http://git.reviewboard.kde.org/r/110082/#comment23365> Please move the return statements to their own line. (though I acknowledge you didn't introduce it) src/browsers/playlistbrowser/PlaylistBrowserModel.cpp <http://git.reviewboard.kde.org/r/110082/#comment23366> again own line please src/core/playlists/Playlist.h <http://git.reviewboard.kde.org/r/110082/#comment23368> I think you've also forgot to remove some description() implementations under podcasts. Or perhaps for podcasts the description is actually useful & used? src/playlistmanager/sql/SqlPlaylist.cpp <http://git.reviewboard.kde.org/r/110082/#comment23367> Although not strictly required, I bet you're be able to do the database update to get rid of the unused column. Please have a look at DatabaseUpdater.cpp, with a bit of SQL knowledge it shouldn't be hard. - Matěj Laitl On April 21, 2013, 12:05 a.m., Vedant Agarwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110082/ > ----------------------------------------------------------- > > (Updated April 21, 2013, 12:05 a.m.) > > > Review request for Amarok. > > > Description > ------- > > As agreed on the review for https://git.reviewboard.kde.org/r/104048/ , > Qt::TooltipRole has been updated so that now the tooltip displays full name > of the playlist. Occurrences of "description" have been removed (from the > Playlist base class as well as the subclasses). > > > This addresses bug 275821. > https://bugs.kde.org/show_bug.cgi?id=275821 > > > Diffs > ----- > > src/playlistmanager/sql/SqlPlaylist.cpp 2d6ef61 > src/playlistmanager/sql/SqlPlaylist.h d28d161 > src/playlistmanager/SyncedPlaylist.cpp 56be7e8 > src/playlistmanager/SyncedPlaylist.h 214bb5c > src/core/playlists/Playlist.h cf9a4c0 > src/core-impl/playlists/types/file/PlaylistFile.cpp 073e140 > src/core-impl/playlists/types/file/PlaylistFile.h 0358196 > > src/core-impl/collections/mediadevicecollection/playlist/MediaDevicePlaylist.cpp > 3510481 > > src/core-impl/collections/mediadevicecollection/playlist/MediaDevicePlaylist.h > 9b94872 > src/browsers/playlistbrowser/PlaylistBrowserModel.cpp 6ee3db3 > > Diff: http://git.reviewboard.kde.org/r/110082/diff/ > > > Testing > ------- > > Testing done. Works. Builds successfully and passes the tests. > > > File Attachments > ---------------- > > displays the new tooltip > > http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/amarok_screenshot1.png > displays the new tooltip > > http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/amarok_screenshot.png > > > Thanks, > > Vedant Agarwala > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
