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

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