Re: Review Request 113272: GSoC 2013 - Advanced Importers - 1/4: Changes in StatSyncing framework

> On Oct. 18, 2013, 10 a.m., Matěj Laitl wrote: > > src/statsyncing/Config.h, line 39 > > > > > > I guess the ability have an extra comma is a g++ extension to C++, > > right? Only thing I like about it is clearer

Re: Review Request 113272: GSoC 2013 - Advanced Importers - 1/4: Changes in StatSyncing framework

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113272/#review41929 --- Ship it! Ship It! - Edward Hades Toroshchin On Oct. 16

Re: Review Request 113277: GSoC 2013 - Advanced Importers - 3/4: Tests for importers framework and concrete importers

571> Probably you should remove private data, like directory path and library ID. - Edward Hades Toroshchin On Oct. 16, 2013, 5:11 p.m., Konrad Zemek wrote: > > --- > This is an automatically generated e-mail. To reply

Re: Review Request 113278: GSoC 2013 - Advanced Importers - 4/4: Other changes in the repository

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113278/#review41888 --- Ship it! Ship It! - Edward Hades Toroshchin On Oct. 16

Re: Review Request 113278: GSoC 2013 - Advanced Importers - 4/4: Other changes in the repository

ttp://git.reviewboard.kde.org/r/113278/#comment30570> I would probably add the "Importer" to the end, just for the off-chance someone attempts to load an old library with old type id. Don't know if it sounds important, just wanted to point out. - Edward Hades Toroshchin On Oct. 16, 20

Re: Review Request 112148: 313504: Prefer QChar overloads over the QString ones for efficiency

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112148/#review38200 --- Thanks, Frank! Awesome job. - Edward Hades Toroshchin On

Re: Review Request 111804: Fix a crash on startup

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111804/#review36908 --- Ship it! but fix the whitespace error first. - Edward Hades

Re: Review Request 111804: Fix a crash on startup

Please check, if this indeed compiles and works. Please also check that the "Shuffle" playlist sort works after Amarok restart. (I believe it isn't in the current solution.) - Edward Hades Toroshchin On July 30, 2013, 8:16 p.m., Fabian Kosmale wrote: > >

Re: Review Request 108717: nepomuk: implement custom QueryMaker

/uploaded/files/2013/02/03/nepomuk-sync.png Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

er hand, there > > are about 265 files that use using namespace XYZ; syntax for function > > definitions. Please improve, rather than fight, consistency. > > Edward Hades Toroshchin wrote: > It's not about consistency. Favouring the using-directive is not a > qu

Re: Review Request 108717: nepomuk: implement custom QueryMaker

individual developers. > > > > There are about 25 .cpp files in current Amarok source code that embody > > all the method definitions into namespace XYZ {}. On the other hand, there > > are about 265 files that use using namespace XYZ; syntax for function &g

Re: Review Request 108717: nepomuk: implement custom QueryMaker

/nepomuk-sync.png Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

ng a favour to your reviewers, > i.e. me. I don't mind that at all. It's the thing you're perfectly right about. - Edward Hades --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.or

Re: Review Request 108717: nepomuk: implement custom QueryMaker

, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

oesn't seem to want to delete it as soon as it goes out of > > scope (and yeah, it may be reset by being on rhs of an assignment, but that > > is convoluted). std::auto_ptr makes perfect sense as a function argument, because it both clearly shows who is responsible for the pointer

Re: Review Request 110658: Playlist sort widget: reimplement Shuffle "sort" as an action.

> Do you really need a nested block here? - Edward Hades Toroshchin On May 27, 2013, 6:21 p.m., Konrad Zemek wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.revi

Re: Review Request 108717: nepomuk: implement custom QueryMaker

at > > /home/mark/Devel/src/amarok/src/EngineController.cpp:342 > > #17 0x7f7e0d1442fe in App::~App (this=0x7fffbaed2118) at > > /home/mark/Devel/src/amarok/src/App.cpp:206 > > #18 0x0040f948 in main (argc=4, argv=0x7fffbaed34b8) at > > /home/mark/Devel/src/amaro

Re: Review Request 110520: create new collections without restart

-- On May 19, 2013, 8:43 a.m., Edward Hades Toroshchin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110520/ > ---

Re: Review Request 108717: nepomuk: implement custom QueryMaker

------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108717/#review32796 --- On May 20, 2013, 10:57 a.m., Edward Hades Toroshchin wrote: > > ---

Re: Review Request 108717: nepomuk: implement custom QueryMaker

://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-sync.png Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-sync.png Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

/02/03/nepomuk-counting-querymaker.png http://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-queries.png http://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-sync.png Thanks, Edward Hades Toroshchin ___ Amarok

Re: Review Request 108717: nepomuk: implement custom QueryMaker

dward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

/02/03/nepomuk-counting-querymaker.png http://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-queries.png http://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-sync.png Thanks, Edward Hades Toroshchin ___ Amarok

Re: Review Request 110187: Don't communicate with mysql by env vars and autogenerated files

oard.kde.org/r/110187/#comment23646> whitespace error - Edward Hades Toroshchin On April 25, 2013, 2:36 p.m., Patrick von Reth wrote: > > --- > This is an automatically generated e-mail. To reply, v

Re: Review Request 110187: Don't communicate with mysql by env vars and autogenerated files

> On April 25, 2013, 3:43 p.m., Edward Hades Toroshchin wrote: > > Thanks for the fix. However, this is the approach, we've used originally, > > and it didn't prove very reliable. I think we even went back and forth more > > than once. > > > > So y

Re: Review Request 110187: Don't communicate with mysql by env vars and autogenerated files

ot only that, it could also lead to problems if several users tried to use Amarok simultaneously. I suggest you check, where does this path come from. Why is storageLocation empty? - Edward Hades Toroshchin On April 25, 2013, 2:36 p.m., Patrick von Reth wrote: > > ---

Re: Review Request 110139: Small bugfixes. Fixes clang warnings.

;. I would personally list all the missing enum values explicitly. Thanks! - Edward Hades Toroshchin On April 24, 2013, 12:25 a.m., Konrad Zemek wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git

Re: Review Request 109157: Amarok buildsystem cleanup preview

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109157/#review28081 --- Why do you want the intermediate libs at all? - Edward Hades

Re: Review Request 108686: hidden items in context menu: usability question

uch things happen: we discussed something on IRC, and Bart wasn't aware of that. Bart, I've committed the hint before the review. Then strohel asked me to open this review to ask the usability folks to help. Which I did. -- Edward "Hades"

Re: Review Request 108686: hidden items in context menu: usability question

f), a file context menu would contain an "Open with..." item only if the Shift key was being held. I think (but 'm not sure) it would also bypass "Trash™" if the user held Shift while clicking "Delete". - Edward Hades ---

Re: Review Request 108717: nepomuk: implement custom QueryMaker

, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

-sync.png Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108717: nepomuk: implement custom QueryMaker

Diff: http://git.reviewboard.kde.org/r/108717/diff/ Testing (updated) --- Playing tracks from Nepomuk collection, browsing, filtering, adding/removing labels. Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org

Re: Review Request 108717: nepomuk: implement custom QueryMaker

1d6e3b1066e13fd6adba15ae6b1d81f8ca28a5d9 Diff: http://git.reviewboard.kde.org/r/108717/diff/ Testing --- Playing tracks from Nepomuk collection, browsing, filtering. Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org

Re: Review Request 108717: nepomuk: implement custom QueryMaker

/collections/nepomukcollection/meta/NepomukYear.cpp 1d6e3b1066e13fd6adba15ae6b1d81f8ca28a5d9 Diff: http://git.reviewboard.kde.org/r/108717/diff/ Testing --- Playing tracks from Nepomuk collection, browsing, filtering. Thanks, Edward Hades Toroshchin

Re: Review Request 108717: nepomuk: implement custom QueryMaker

/collections/nepomukcollection/meta/NepomukYear.cpp 1d6e3b1066e13fd6adba15ae6b1d81f8ca28a5d9 Diff: http://git.reviewboard.kde.org/r/108717/diff/ Testing --- Playing tracks from Nepomuk collection, browsing, filtering. Thanks, Edward Hades Toroshchin

Re: Review Request 108717: nepomuk: implement custom QueryMaker

/nepomukcollection/meta/NepomukYear.cpp 1d6e3b1066e13fd6adba15ae6b1d81f8ca28a5d9 Diff: http://git.reviewboard.kde.org/r/108717/diff/ Testing --- Playing tracks from Nepomuk collection, browsing, filtering. Thanks, Edward Hades Toroshchin

Re: Review Request: Changes in processing playlist files

> 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

> 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: Statistics Synchronization: final review request

ard, when you've already compiled the StatSyncing code, perhaps you > can lightly test it? ;) UI in Amarok Config, scrobbling still working, > synchronizing with Last.fm (warning: non-interactive part is time-consuming) > or with an iPod... Sure, just don't wait for me b

Re: Review Request: Statistics Synchronization: final review request

on in StatSync::Provider, and according to yourself. And it won't make it useless. Again, your Provider is in main thread, but it isn't useless, despite that it's essentially a blocking interface to QueryMaker. -- Edward "Hades" Toroshchin dr_lepper on irc.freenode.org __

Re: Review Request: Statistics Synchronization: final review request

e it on my side. Well, if you document the diamond inheritance for people who have heard first of it, you could've documented this "pattern" here as well. -- Edward "Hades" Toroshchin dr_lepper on irc.freenode.org ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request: Statistics Synchronization: final review request

> On Nov. 17, 2012, 7:59 p.m., Edward Hades Toroshchin wrote: > > Haven't run it yet, but at least it compiles okay :) > > Matěj Laitl wrote: > Thanks for the attentive review, I didn't expect somebody to dive that > deep and I'm grateful. > >

Re: Review Request: Statistics Synchronization: final review request

> On Nov. 17, 2012, 7:59 p.m., Edward Hades Toroshchin wrote: > > src/services/lastfm/SynchronizationAdapter.cpp, lines 38-43 > > <http://git.reviewboard.kde.org/r/107348/diff/1/?file=94955#file94955line38> > > > > This should be Qt::AutoConnection. Otherwise

Re: Review Request: collectionscanner: prevent writing malformed XML

. Looks fine. Scanned some folders with creepy characters. Looks also fine. Thanks, Edward Hades Toroshchin ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request: GSoC : Nepomuk plugin for Amarok

e configure which folders Nepomuk > > indexes? You should use these for this call. (not a merge-blocker) > > Edward Hades Toroshchin wrote: > Phalgun, add a // TODO comment here, so that this can be investigated in > the future. > > Bart Cerneels wrote: > You shoul

Re: Review Request: GSoC : Nepomuk plugin for Amarok

rd.kde.org/r/106042/#review18515 --- On Sept. 4, 2012, 4:51 p.m., Phalgun Guduthur wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106042/ >

Re: Review Request: GSoC : Nepomuk plugin for Amarok

sign issue: > > > > You either need MemoryCollection or all these hashes, but not both. I'm > > still not convinced that you cannot avoid MemoryCollection at all, but if > > you use it, don't duplicate MapChanger. > > Edward Hades Toroshchin wrote:

Re: Review Request: GSoC : Nepomuk plugin for Amarok

sign issue: > > > > You either need MemoryCollection or all these hashes, but not both. I'm > > still not convinced that you cannot avoid MemoryCollection at all, but if > > you use it, don't duplicate MapChanger. > > Edward Hades Toroshchin wrote:

Re: Review Request: GSoC : Nepomuk plugin for Amarok

sign issue: > > > > You either need MemoryCollection or all these hashes, but not both. I'm > > still not convinced that you cannot avoid MemoryCollection at all, but if > > you use it, don't duplicate MapChanger. > > Edward Hades Toroshchin wrote:

Re: Review Request: GSoC : Nepomuk plugin for Amarok

- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review1 --- On Aug. 20, 2012, 11:17 a.m., Phalgun Guduthur wrote: > > --- > T

Re: Review Request: GSoC : Nepomuk plugin for Amarok

.m., Phalgun Guduthur wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106042/ > --- > > (Updated Aug. 1

Re: Review Request: {Smb, Nfs}DeviceHandler: use Solid instead of KMountPoint

/NfsDeviceHandler.cpp <http://git.reviewboard.kde.org/r/106094/#comment13907> Are you sure it couldn't have been done with QUrl::host() and QUrl::path()? src/core-impl/collections/db/sql/device/smb/SmbDeviceHandler.cpp <http://git.reviewboard.kde.org/r/106094/#comment13908> Same here.

Re: Review Request: GSoC : Nepomuk plugin for Amarok

nice progress bar here, by using Amarok::Components::Logger->newProgressOperation() (although I'm not quite sure it works immediately with ThreadWeaver jobs) - Edward Hades Toroshchin On Aug. 16, 2012, 3:52 p.m., Phalgun Guduthur wrote: > > ---

Review Request: magnatune: first update related tweaks

; b) If the "update automatically" checkbox is set, it checks for updates automatically. Screenshots --- the first-run widget http://git.reviewboard.kde.org/r/106071/s/680/ Thanks, Edward Hades Toroshchin ___ Amarok-devel mailin

Re: Review Request: Proof of concept for the 2012 GSoC project idea 'Semantic Collection for Amarok'.

> On Aug. 16, 2012, 12:28 p.m., Ralf Engels wrote: > > I wouldn't ship it. > > Ratings will not longer be written back to the files. Search would not work > > (since the search is done in the sql database and the rating is not longer > > written there) and so on. > > How about a real Nepumuk ba

Re: Review Request: Do not return collectionFolders() unless ready.

to do nothing, which is accomplished by returning empty list. - Edward Hades --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105246/#review14729 ---------

changing rating in playlist inline editor

iting. This however would not please the guy at bug 223309 [1] and other old-amarok fans, because they would also like to remove items #1 and #2 (and I personally see their point), but I do not think it is very easy to implement. What do you think? Best, -- Edward "Hades" Toroshchin

Review Request: Do not create default APG presets based on filename

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100372/ --- Review request for Amarok and Soren Harward. Summary --- Currently AP