Re: Review Request 108716: FIX Randomize playlist with Ctrl+H
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108716/ --- (Updated Feb. 3, 2013, 10:04 p.m.) Review request for Amarok. Description --- Added a shortcut button for shuffle with default shortcut as Ctrl + H. Created a slot shuffleTrack() in mainWindow.cpp which is triggered on pressing Ctrl + H Diffs - src/MainWindow.h 4b23679 src/MainWindow.cpp 8587784 Diff: http://git.reviewboard.kde.org/r/108716/diff/ Testing (updated) --- All tests passed. File Attachments http://git.reviewboard.kde.org/media/uploaded/files/2013/02/02/0001-Randomize-playlist-with-Ctrl-H.patch Thanks, Harsh Gupta ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
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 --- Refactoring playlists. This patch also includes: 1) Bug in loading default playlist was fixed. 2) Logic for restoreDefaultPlaylist was rewritten. Testing: 1) First start up loads default tune. 2) Default playlist is restored successfully. Remark: I do not know how to generate playlist which can contain other playlists, so I was unable to test this situation. Description --- IMPORTANT: I have found bug in last implementation. Please postpone review until I fix it. 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 (updated) - src/core-impl/playlists/types/file/PlaylistFile.h 4322da9 src/core-impl/playlists/types/file/PlaylistFile.cpp 2730354 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 3ba154a src/core-impl/playlists/types/file/xspf/XSPFPlaylist.h ebf3235 src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp bbcecc3 src/core/playlists/Playlist.h 8fd1ffb src/playlist/PlaylistActions.h d57a5c9 src/playlist/PlaylistActions.cpp 13810cd src/playlistmanager/PlaylistManager.h cbf65b0 src/playlistmanager/PlaylistManager.cpp 89c754b src/playlistmanager/file/PlaylistFileProvider.cpp e4e7284 tests/core-impl/playlists/types/file/CMakeLists.txt ef69236 tests/core-impl/playlists/types/file/TestPlaylistObserver.h PRE-CREATION tests/core-impl/playlists/types/file/TestPlaylistObserver.cpp PRE-CREATION tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp 0f19e1b tests/core-impl/playlists/types/file/xspf/TestXSPFPlaylist.cpp f4643ca 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 ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
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 (updated) --- 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/core-impl/playlists/types/file/PlaylistFile.h 4322da9 src/core-impl/playlists/types/file/PlaylistFile.cpp 2730354 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 3ba154a src/core-impl/playlists/types/file/xspf/XSPFPlaylist.h ebf3235 src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp bbcecc3 src/core/playlists/Playlist.h 8fd1ffb src/playlist/PlaylistActions.h d57a5c9 src/playlist/PlaylistActions.cpp 13810cd src/playlistmanager/PlaylistManager.h cbf65b0 src/playlistmanager/PlaylistManager.cpp 89c754b src/playlistmanager/file/PlaylistFileProvider.cpp e4e7284 tests/core-impl/playlists/types/file/CMakeLists.txt ef69236 tests/core-impl/playlists/types/file/TestPlaylistObserver.h PRE-CREATION tests/core-impl/playlists/types/file/TestPlaylistObserver.cpp PRE-CREATION tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp 0f19e1b tests/core-impl/playlists/types/file/xspf/TestXSPFPlaylist.cpp f4643ca 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 ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Build failed in Jenkins: amarok_master #232
See http://build.kde.org/job/amarok_master/232/ -- Started by remote host 127.0.0.1 with note: Triggered by commit Building remotely on LinuxSlave - 2 in workspace http://build.kde.org/job/amarok_master/ws/ Running Prebuild steps [amarok_master] $ /bin/sh -xe /tmp/hudson6238136497417339736.sh + /home/jenkins/scripts/setup-env.sh Traceback (most recent call last): File tools/prepare-environment.py, line 39, in module ProjectManager.load_projects( etree.parse(fileHandle) ) File lxml.etree.pyx, line 2942, in lxml.etree.parse (src/lxml/lxml.etree.c:54187) File parser.pxi, line 1550, in lxml.etree._parseDocument (src/lxml/lxml.etree.c:79703) File parser.pxi, line 1580, in lxml.etree._parseFilelikeDocument (src/lxml/lxml.etree.c:80012) File parser.pxi, line 1463, in lxml.etree._parseDocFromFilelike (src/lxml/lxml.etree.c:78908) File parser.pxi, line 1019, in lxml.etree._BaseParser._parseDocFromFilelike (src/lxml/lxml.etree.c:75905) File parser.pxi, line 564, in lxml.etree._ParserContext._handleParseResultDoc (src/lxml/lxml.etree.c:71739) File parser.pxi, line 645, in lxml.etree._handleParseResult (src/lxml/lxml.etree.c:72614) File parser.pxi, line 585, in lxml.etree._raiseParseError (src/lxml/lxml.etree.c:71955) lxml.etree.XMLSyntaxError: Premature end of data in tag project line 3611, line 3619, column 1 Failed build for hudson.tasks.Shell@5ba9d23 Checkout:amarok_master / http://build.kde.org/job/amarok_master/ws/ - hudson.remoting.Channel@48078f54:LinuxSlave - 2 Using strategy: Default Last Built Revision: Revision ec90991276118972617b24124ee9b0a6f7e2bde4 (refs/heads/jenkins) Fetching changes from 1 remote Git repository Fetching upstream changes from git://anongit.kde.org/amarok.git Commencing build of Revision ec90991276118972617b24124ee9b0a6f7e2bde4 (refs/heads/jenkins) Checking out Revision ec90991276118972617b24124ee9b0a6f7e2bde4 (refs/heads/jenkins) [amarok_master] $ /bin/sh -xe /tmp/hudson360123567814332645.sh + /home/jenkins/scripts/execute-job.sh Traceback (most recent call last): File tools/perform-build.py, line 38, in module ProjectManager.load_projects( etree.parse(fileHandle) ) File lxml.etree.pyx, line 2942, in lxml.etree.parse (src/lxml/lxml.etree.c:54187) File parser.pxi, line 1550, in lxml.etree._parseDocument (src/lxml/lxml.etree.c:79703) File parser.pxi, line 1580, in lxml.etree._parseFilelikeDocument (src/lxml/lxml.etree.c:80012) File parser.pxi, line 1463, in lxml.etree._parseDocFromFilelike (src/lxml/lxml.etree.c:78908) File parser.pxi, line 1019, in lxml.etree._BaseParser._parseDocFromFilelike (src/lxml/lxml.etree.c:75905) File parser.pxi, line 564, in lxml.etree._ParserContext._handleParseResultDoc (src/lxml/lxml.etree.c:71739) File parser.pxi, line 645, in lxml.etree._handleParseResult (src/lxml/lxml.etree.c:72614) File parser.pxi, line 585, in lxml.etree._raiseParseError (src/lxml/lxml.etree.c:71955) lxml.etree.XMLSyntaxError: Premature end of data in tag project line 3611, line 3619, column 1 Build step 'Execute shell' marked build as failure [WARNINGS] Skipping publisher since build result is FAILURE Recording test results ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
