Review Request 110082: fixes Bug 275821 - JJ: Proper tooltips for Saved Playlists; remove Playlist::description() method

[Matěj Laitl]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130602/f61f828f/attachment-0001.html>