Review Request 107473: Changes in processing playlist files

[Matěj Laitl]

> On March 27, 2013, 12:35 p.m., Matěj Laitl wrote:
> > Yeah, nice change, you go even further with code deduplication. I have a load of changes that should be applied on top of your patch (which will include fixes to issues below), please wait for it along with some comments.

You also accidentally forgot to include 6 newly added files in the patch v10, please reinclude them ASAP (so that I can rebase my changes on top of it).


- Matěj


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107473/#review29948
-----------------------------------------------------------


On March 27, 2013, 1:13 a.m., Tatjana Gornak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107473/
> -----------------------------------------------------------
> 
> (Updated March 27, 2013, 1:13 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> 
> I've started my changes with an implementation of a ASXPlaylist class,
> due this work I've found that implementation of different playlist file
> types has different logic, as result I end up with a rewriting
> playlist's implementations.
> 
> As example of difference:
>   -- Constructor M3UPlaylist::M3UPlaylist( const KUrl &url ) sets a url, but
>      does not load playlist, therefore loading of playlist are
> posponed till data from playlist are actualy needed (lazy loading)
>      On the other hand constructor XSPFPlaylist::XSPFPlaylist( const
> KUrl &url, bool autoAppend )  loads playlist.
> 
> The main idea of proposed changes is to create a unify code for
> processing playlist files:
>   -- lazy loading through LazyLoadPlaylist
>   -- common functionality was moved to PlaylistFile.
> 
> It is worth to be noticed that this patch changes structure of
> playlist's classes, bit does not changes their behavior, even if this behavior
> is inconsistent in some cases.
> 
> In following patch-requests I plan to submit ASX Playlist parser and
> organize playlists processing in more consistent way.
> 
> 
> This addresses bug 291934.
>     https://bugs.kde.org/show_bug.cgi?id=291934
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt c03541f 
>   src/DirectoryLoader.h e53a30d 
>   src/DirectoryLoader.cpp bd7fa34 
>   src/browsers/playlistbrowser/PlaylistBrowserModel.h 1b7a44d 
>   src/browsers/playlistbrowser/PlaylistBrowserModel.cpp 213af3b 
>   src/core-impl/collections/ipodcollection/IpodPlaylistProvider.h 4dd79f3 
>   src/core-impl/collections/ipodcollection/IpodPlaylistProvider.cpp abf0fae 
>   src/core-impl/playlists/types/file/PlaylistFile.h 43dfdeb 
>   src/core-impl/playlists/types/file/PlaylistFile.cpp 2730354 
>   src/core-impl/playlists/types/file/PlaylistFileSupport.h b1a1c26 
>   src/core-impl/playlists/types/file/PlaylistFileSupport.cpp 2dcc0cd 
>   src/core-impl/playlists/types/file/m3u/M3UPlaylist.h 3176c45 
>   src/core-impl/playlists/types/file/m3u/M3UPlaylist.cpp 2395260 
>   src/core-impl/playlists/types/file/pls/PLSPlaylist.h e974539 
>   src/core-impl/playlists/types/file/pls/PLSPlaylist.cpp 8fe4b51 
>   src/core-impl/playlists/types/file/xspf/XSPFPlaylist.h 6cf33e9 
>   src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp bbcecc3 
>   src/core-impl/podcasts/sql/SqlPodcastMeta.cpp 1a025ce 
>   src/core/playlists/Playlist.h 018d2af 
>   src/core/playlists/Playlist.cpp 36879a1 
>   src/playlist/PlaylistActions.cpp 00bf13a 
>   src/playlist/PlaylistController.cpp e5bde8b 
>   src/playlistmanager/PlaylistManager.h cbf65b0 
>   src/playlistmanager/PlaylistManager.cpp 89c754b 
>   src/playlistmanager/SyncedPlaylist.h fce3d14 
>   src/playlistmanager/SyncedPlaylist.cpp 4f7c0b7 
>   src/playlistmanager/file/PlaylistFileProvider.cpp e4e7284 
>   src/playlistmanager/sql/SqlPlaylist.cpp 88e685b 
>   tests/TestDirectoryLoader.h 0583a9e 
>   tests/TestDirectoryLoader.cpp b98247d 
>   tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp b25d059 
>   tests/core-impl/playlists/types/file/pls/TestPLSPlaylist.h 1183599 
>   tests/core-impl/playlists/types/file/pls/TestPLSPlaylist.cpp 5eab2f3 
>   tests/core-impl/playlists/types/file/xspf/TestXSPFPlaylist.cpp 5ea3a41 
>   tests/core/playlists/CMakeLists.txt e2e0ce4 
> 
> Diff: http://git.reviewboard.kde.org/r/107473/diff/
> 
> 
> Testing
> -------
> 
> 1) All unit-tests were passed.
> 2) For all playlists I've also checked loading and
>    saving through gui.
> 
> 
> Thanks,
> 
> Tatjana Gornak
> 
>

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