Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review30269 --- Ship it! Good work! Now I'll be happy to merge the updated ve

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated April 2, 2013, 10:47 a.m.) Status -- This change has been ma

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review30255 --- This review has been submitted with commit 6f87755c9ab292dc0c5

Re: Review Request 107473: Changes in processing playlist files

> On April 2, 2013, 7:09 a.m., Bart Cerneels wrote: > > A suggestion: start a feature branch to work on this. Mucking about with > > patches is not very efficient. I think you 2 can discuss the code in > > details without the help of reviewboard by now. And before merging you can > > upload it

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review30244 --- A suggestion: start a feature branch to work on this. Mucking a

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated April 1, 2013, 6:57 p.m.) Review request for Amarok. Changes --

Re: Review Request 107473: Changes in processing playlist files

> On March 28, 2013, 12:16 a.m., Matěj Laitl wrote: > > Hmmm, I'm apparently able to update the diff, please disregard the attached > > file. > > > > Please have a look a differences between v11 and v12, most of them are > > style fixes or cleanups, but it also boasts fixes for a couple of sub

Re: Review Request 107473: Changes in processing playlist files

> On March 28, 2013, 12:16 a.m., Matěj Laitl wrote: > > Hmmm, I'm apparently able to update the diff, please disregard the attached > > file. > > > > Please have a look a differences between v11 and v12, most of them are > > style fixes or cleanups, but it also boasts fixes for a couple of sub

Re: Review Request 107473: Changes in processing playlist files

> On March 28, 2013, 12:16 a.m., Matěj Laitl wrote: > > Hmmm, I'm apparently able to update the diff, please disregard the attached > > file. > > > > Please have a look a differences between v11 and v12, most of them are > > style fixes or cleanups, but it also boasts fixes for a couple of sub

Re: Review Request 107473: Changes in processing playlist files

> On March 28, 2013, 12:16 a.m., Matěj Laitl wrote: > > Hmmm, I'm apparently able to update the diff, please disregard the attached > > file. > > > > Please have a look a differences between v11 and v12, most of them are > > style fixes or cleanups, but it also boasts fixes for a couple of sub

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review29971 --- Hmmm, I'm apparently able to update the diff, please disregard

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated March 27, 2013, 7:35 p.m.) Review request for Amarok. Changes -

Re: Review Request 107473: Changes in processing playlist files

> On March 27, 2013, 12:35 p.m., Matěj Laitl wrote: > > src/core-impl/playlists/types/file/m3u/M3UPlaylist.h, lines 49-52 > > > > > > Private? Should be "protected". > > Tatjana Gornak wrote: > Sorry, origi

Re: Review Request 107473: Changes in processing playlist files

> On March 27, 2013, 12:35 p.m., Matěj Laitl wrote: > > src/core-impl/playlists/types/file/m3u/M3UPlaylist.h, lines 49-52 > > > > > > Private? Should be "protected". Sorry, originally I've planned to make Playl

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated March 27, 2013, 3:41 p.m.) Review request for Amarok. Changes -

Re: Review Request 107473: Changes in processing playlist files

> 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

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review29948 --- Yeah, nice change, you go even further with code deduplication.

Re: Review Request 107473: Changes in processing playlist files

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

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated March 17, 2013, 5:34 a.m.) Review request for Amarok. Changes -

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated March 14, 2013, 6:19 p.m.) Review request for Amarok. Descripti

Re: Review Request 107473: Changes in processing playlist files

> On March 11, 2013, 12:28 p.m., Matěj Laitl wrote: > > src/core/playlists/Playlist.h, lines 260-263 > > > > > > I think you're down to a very small amount of subclasses, as affected > > are ones that implement t

Re: Review Request 107473: Changes in processing playlist files

> On March 5, 2013, 12:11 a.m., Matěj Laitl wrote: > > src/DirectoryLoader.cpp, lines 168-169 > > > > > > Here you would have to deal with both async-loading and sync-loading > > playlists (you don't want to add

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated March 11, 2013, 1:57 a.m.) Review request for Amarok. Descripti

Re: Review Request 107473: Changes in processing playlist files

> On March 5, 2013, 12:11 a.m., Matěj Laitl wrote: > > src/core/playlists/Playlist.h, lines 201-207 > > > > > > This is too specific to warrant 2 new methods and a class variable. > > Please add "bool asynchronou

Re: Review Request 107473: Changes in processing playlist files

> On March 5, 2013, 12:11 a.m., Matěj Laitl wrote: > > Tatjana, thanks for your continued effort on this review. :-) There seem to > > be a couple of problematic places that may cause regressions, please take a > > look at them so this can be merged. (which I hope can be done rather soon, > >

Re: Review Request 107473: Changes in processing playlist files

> On March 5, 2013, 12:11 a.m., Matěj Laitl wrote: > > src/playlist/PlaylistRestorer.h, line 56 > > > > > > Class-wise this is redundant with m_tracks and should be avoided. Just > > create local iterator in proce

Re: Review Request 107473: Changes in processing playlist files

> On March 5, 2013, 12:11 a.m., Matěj Laitl wrote: > > src/DirectoryLoader.cpp, lines 168-169 > > > > > > Here you would have to deal with both async-loading and sync-loading > > playlists (you don't want to add

Re: Review Request 107473: Changes in processing playlist files

> On March 5, 2013, 12:11 a.m., Matěj Laitl wrote: > > Tatjana, thanks for your continued effort on this review. :-) There seem to > > be a couple of problematic places that may cause regressions, please take a > > look at them so this can be merged. (which I hope can be done rather soon, > >

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated March 2, 2013, 6:06 p.m.) Review request for Amarok. Changes --

Re: Review Request 107473: Changes in processing playlist files

> On Feb. 9, 2013, 12:55 a.m., Matěj Laitl wrote: > > src/core-impl/playlists/types/file/PlaylistFile.h, lines 71-73 > > > > > > What is the difference between save() and savePlaylist()? Also, if we > > don't allo

Re: Review Request 107473: Changes in processing playlist files

> On Feb. 9, 2013, 12:55 a.m., Matěj Laitl wrote: > > src/core-impl/playlists/types/file/PlaylistFile.h, lines 86-93 > > > > > > Why are the 2 very similar methods needed? I'd like if they could be > > factored in

Re: Review Request 107473: Changes in processing playlist files

> On Feb. 9, 2013, 12:55 a.m., Matěj Laitl wrote: > > src/core-impl/playlists/types/file/PlaylistFile.h, lines 71-73 > > > > > > What is the difference between save() and savePlaylist()? Also, if we > > don't allo

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated Feb. 7, 2013, 3:57 p.m.) Review request for Amarok. Changes ---

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated Feb. 3, 2013, 6:39 p.m.) Review request for Amarok. Description

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated Feb. 3, 2013, 6:39 p.m.) Review request for Amarok. Changes ---

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated Feb. 2, 2013, 4:18 p.m.) Review request for Amarok. Description

Re: Review Request 107473: Changes in processing playlist files

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated Jan. 15, 2013, 9:21 p.m.) Review request for Amarok. Changes --