Re: Review Request: Changes in processing playlist files

2013-01-11 Thread Tatjana Gornak
> On Jan. 2, 2013, 7:59 a.m., Bart Cerneels wrote: > > src/core-impl/playlists/types/file/PlaylistFile.cpp, line 238 > > > > > > Have you looked at loading playlists in batches in PlaylistFileProvider > > instead

Re: Review Request: Changes in processing playlist files

2013-01-03 Thread Tatjana Gornak
> On Jan. 2, 2013, 7:59 a.m., Bart Cerneels wrote: > > src/core-impl/playlists/types/file/PlaylistFile.h, line 137 > > > > > > Having to use mutable always makes me suspicious of architectural > > mistakes. But I

Re: Review Request: Changes in processing playlist files

2013-01-03 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated Jan. 3, 2013, 12:34 p.m.) Review request for Amarok. Changes --

Re: Review Request: Changes in processing playlist files

2013-01-01 Thread Bart Cerneels
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review24408 --- src/core-impl/playlists/types/file/PlaylistFile.h

Re: Review Request: Changes in processing playlist files

2013-01-01 Thread Bart Cerneels
> On Nov. 27, 2012, 8:08 a.m., Bart Cerneels wrote: > > src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp, line 218 > > > > > > Here is the real reason why lazy loading is needed. Reading > > playlist-file c

Re: Review Request: Changes in processing playlist files

2013-01-01 Thread Bart Cerneels
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review24407 --- Looks like I still had some unpublished comment from the last v

Re: Review Request: Changes in processing playlist files

2013-01-01 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review24399 --- Hi, thanks for the updated revision! As this is a bigger one, p

Re: Review Request: Changes in processing playlist files

2012-12-29 Thread Tatjana Gornak
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- (Updated Dec. 29, 2012, 12:46 p.m.) Review request for Amarok. Changes -

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Matěj Laitl
> On Nov. 26, 2012, 6:16 p.m., Matěj Laitl wrote: > > Hi, thanks for your work! > > > > First, I must acknowledge that our current file playlist code is a big mess > > and it certainly needs cleanup and code deduplication, your work is > > therefore greatly appreciated. I have reviewed just on

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Edward Hades Toroshchin
> On Nov. 26, 2012, 6:16 p.m., Matěj Laitl wrote: > > Hi, thanks for your work! > > > > First, I must acknowledge that our current file playlist code is a big mess > > and it certainly needs cleanup and code deduplication, your work is > > therefore greatly appreciated. I have reviewed just on

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Matěj Laitl
> On Nov. 26, 2012, 6:16 p.m., Matěj Laitl wrote: > > Hi, thanks for your work! > > > > First, I must acknowledge that our current file playlist code is a big mess > > and it certainly needs cleanup and code deduplication, your work is > > therefore greatly appreciated. I have reviewed just on

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Edward Hades Toroshchin
> On Nov. 27, 2012, 8:08 a.m., Bart Cerneels wrote: > > src/core-impl/playlists/types/file/LazyPlaylistFile.cpp, lines 22-38 > > > > > > I'm surprised your compiler does not trip up on this. Better remove the > > ';

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Matěj Laitl
> On Nov. 27, 2012, 8:08 a.m., Bart Cerneels wrote: > > src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp, line 218 > > > > > > Here is the real reason why lazy loading is needed. Reading > > playlist-file c

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Bart Cerneels
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review22631 --- This architecture document should help and avoid me repeating m

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Bart Cerneels
> On Nov. 27, 2012, 8:08 a.m., Bart Cerneels wrote: > > I think you started with the wrong assumption that lazy loading of the > > contents of the playlist file is required. > > We have some use cases for PlaylistFile: > > 1) Make playlists found in the collection folders available through the

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Matěj Laitl
> On Nov. 27, 2012, 8:08 a.m., Bart Cerneels wrote: > > I think you started with the wrong assumption that lazy loading of the > > contents of the playlist file is required. > > We have some use cases for PlaylistFile: > > 1) Make playlists found in the collection folders available through the

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Myriam Schweingruber
> On Nov. 27, 2012, 8:08 a.m., Bart Cerneels wrote: > > src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp, line 218 > > > > > > Here is the real reason why lazy loading is needed. Reading > > playlist-file c

Re: Review Request: Changes in processing playlist files

2012-11-27 Thread Bart Cerneels
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review22621 --- I think you started with the wrong assumption that lazy loading

Re: Review Request: Changes in processing playlist files

2012-11-26 Thread Tatjana Gornak
> On Nov. 26, 2012, 6:16 p.m., Matěj Laitl wrote: > > Hi, thanks for your work! > > > > First, I must acknowledge that our current file playlist code is a big mess > > and it certainly needs cleanup and code deduplication, your work is > > therefore greatly appreciated. I have reviewed just on

Re: Review Request: Changes in processing playlist files

2012-11-26 Thread Matěj Laitl
> On Nov. 26, 2012, 6:16 p.m., Matěj Laitl wrote: > > Hi, thanks for your work! > > > > First, I must acknowledge that our current file playlist code is a big mess > > and it certainly needs cleanup and code deduplication, your work is > > therefore greatly appreciated. I have reviewed just on

Re: Review Request: Changes in processing playlist files

2012-11-26 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review22586 --- Hi, thanks for your work! First, I must acknowledge that our c

Review Request: Changes in processing playlist files

2012-11-26 Thread Tatjana Gornak
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/ --- Review request for Amarok. Description --- I've started my changes wi