----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110082/#review33593 -----------------------------------------------------------
Hi, it seems that some problems fixed in v3 leaked into v4, please fix these. There are also some minor things in the v4. src/browsers/playlistbrowser/PlaylistBrowserModel.cpp <http://git.reviewboard.kde.org/r/110082/#comment24820> This is a regression with regards to v3 of the patch. src/browsers/playlistbrowser/PlaylistBrowserModel.cpp <http://git.reviewboard.kde.org/r/110082/#comment24821> Regression wrt v3 of the patch. src/playlistmanager/sql/SqlUserPlaylistProvider.cpp <http://git.reviewboard.kde.org/r/110082/#comment24823> What about my comment that this should be documented that it needs to appear right before "case N: // current version"? src/playlistmanager/sql/SqlUserPlaylistProvider.cpp <http://git.reviewboard.kde.org/r/110082/#comment24822> This needs to be even more visible, I suggest using KMessageBox::sorry(), you actually need to replace the %1 and %2 in i18n() calls! - Matěj Laitl On May 20, 2013, 9:48 p.m., Vedant Agarwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110082/ > ----------------------------------------------------------- > > (Updated May 20, 2013, 9:48 p.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/browsers/playlistbrowser/PlaylistBrowserModel.cpp d2b55ff > > src/core-impl/collections/mediadevicecollection/playlist/MediaDevicePlaylist.h > 6b25e59 > > src/core-impl/collections/mediadevicecollection/playlist/MediaDevicePlaylist.cpp > 1ad4d55 > src/core-impl/playlists/types/file/PlaylistFile.h bd88199 > src/core-impl/playlists/types/file/PlaylistFile.cpp 073e140 > src/core/playlists/Playlist.h 39ecb30 > src/playlistmanager/SyncedPlaylist.h fd2f966 > src/playlistmanager/SyncedPlaylist.cpp 985f087 > src/playlistmanager/sql/SqlPlaylist.h d28d161 > src/playlistmanager/sql/SqlPlaylist.cpp 98f24d2 > src/playlistmanager/sql/SqlPlaylistGroup.cpp 2862034 > src/playlistmanager/sql/SqlUserPlaylistProvider.h 273a050 > src/playlistmanager/sql/SqlUserPlaylistProvider.cpp d9209d2 > > 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
