Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-17 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review19087 --- Please resolve these minor issues, then merge into your master

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-17 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Sept. 17, 2012, 6:06 p.m.) Review request for Amarok, Edward Hade

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-16 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review19018 --- I have just found this issue. I'll be happier if you could ditc

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-14 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Sept. 15, 2012, 4:22 a.m.) Review request for Amarok, Edward Hade

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-14 Thread Phalgun Guduthur
> On Sept. 13, 2012, 7:43 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, > > lines 210-216 > > > > > > My reasoning about the KSharedPtrs: > > > > You

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-14 Thread Matěj Laitl
> On Sept. 13, 2012, 7:43 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, > > lines 151-154 > > > > > > What about my comments about moving this above the main

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-13 Thread Phalgun Guduthur
> On Sept. 13, 2012, 7:43 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, > > lines 151-154 > > > > > > What about my comments about moving this above the main

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-13 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review18939 --- Looks better now. I was a bit confused about my previous commen

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-13 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Sept. 13, 2012, 6:58 p.m.) Review request for Amarok, Edward Hade

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-13 Thread Phalgun Guduthur
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/meta/NepomukArtist.h, line 27 > > > > > > Not needed at all IMO. > > Phalgun Guduthur wrote: > I'm using these KS

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-11 Thread Phalgun Guduthur
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukCollection.h, line 58 > > > > > > See my previous comments on this line and accordingly. NepomukCollection sti

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-11 Thread Phalgun Guduthur
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/meta/NepomukAlbum.h, line 38 > > > > > > Just accept Meta::ArtistPtr for artist (od QString and construct > > Nepomuk

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-10 Thread Phalgun Guduthur
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, > > line 167 > > > > > > length is int, why you use toDouble() for it? (same applies to

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-10 Thread Phalgun Guduthur
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, > > line 260 > > > > > > Does MemoryMeta handle labels right? So I should be duplicati

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-05 Thread Phalgun Guduthur
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukCollection.cpp, line 124 > > > > > > This works for now, but cannot one configure which folders Nepomuk > > i

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-05 Thread Edward Hades Toroshchin
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukCollection.cpp, line 124 > > > > > > This works for now, but cannot one configure which folders Nepomuk > > i

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-05 Thread Phalgun Guduthur
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukCollection.cpp, line 124 > > > > > > This works for now, but cannot one configure which folders Nepomuk > > i

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-04 Thread Bart Cerneels
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukCollection.h, line 42 > > > > > > Nicpick: instead -> "in addition to" It actually is the goal of the nepomuk

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-04 Thread Matěj Laitl
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/meta/NepomukTrack.h, line 114 > > > > > > No. "This should be implemented whethever you return true in > > inCollect

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-04 Thread Edward Hades Toroshchin
> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukCollection.cpp, line 124 > > > > > > This works for now, but cannot one configure which folders Nepomuk > > i

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-04 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Sept. 4, 2012, 4:51 p.m.) Review request for Amarok, Edward Hades

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-09-04 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Sept. 4, 2012, 4:44 p.m.) Review request for Amarok, Edward Hades

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-27 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 27, 2012, 5:50 p.m.) Review request for Amarok, Edward Hades

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-26 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 26, 2012, 6:40 p.m.) Review request for Amarok, Edward Hades

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-24 Thread Phalgun Guduthur
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-24 Thread Matěj Laitl
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-24 Thread Matěj Laitl
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-23 Thread Phalgun Guduthur
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-23 Thread Bart Cerneels
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukCollection.cpp, line 281 > > > > > > Register the job with Amarok::Components::logger(), mentioning its > >

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Bart Cerneels
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Edward Hades Toroshchin
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Edward Hades Toroshchin
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Matěj Laitl
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Edward Hades Toroshchin
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review17849 --- src/core-impl/collections/nepomukcollection/NepomukConstructMe

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Matěj Laitl
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h, line > > 71 > > > > > > This is an important design issue: > > > > You either ne

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-22 Thread Edward Hades Toroshchin
> On Aug. 20, 2012, 10:46 p.m., Matěj Laitl wrote: > > src/core-impl/collections/nepomukcollection/CMakeLists.txt, line 1 > > > > > > 1. This should be rather macro_optional_find_package( Nepomuk ) to > > allow users

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-20 Thread Phalgun Guduthur
> On Aug. 20, 2012, 9:23 a.m., Vishesh Handa wrote: > > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, > > line 203 > > > > > > I'm slightly confused. > > > > Why does the track c

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-20 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 20, 2012, 11:17 a.m.) Review request for Amarok, Edward Hade

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-20 Thread Vishesh Handa
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review17744 --- This is just the first cursory glance. I'll look into it more l

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-20 Thread Edward Hades Toroshchin
> On Aug. 20, 2012, 9:23 a.m., Vishesh Handa wrote: > > This is just the first cursory glance. I'll look into it more later. > > > > I really love review-board. We should have done this during the entire > > gsoc. :) I hate it :( > On Aug. 20, 2012, 9:23 a.m., Vishesh Handa wrote: > > src/co

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-18 Thread Phalgun Guduthur
> On Aug. 15, 2012, 5:23 p.m., Matěj Laitl wrote: > > src/core-impl/collections/support/MemoryMeta.cpp, line 148 > > > > > > Good. Please have this change as a separate commit (perhaps it already > > is, I haven't

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-18 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 18, 2012, 5:58 p.m.) Review request for Amarok, Edward Hades

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-18 Thread Edward Hades Toroshchin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review17660 --- Ship it! Can't see anything wrong with this. Good job! Ship

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-16 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 16, 2012, 3:52 p.m.) Review request for Amarok, Edward Hades

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-15 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 15, 2012, 6:55 p.m.) Review request for Amarok, Vishesh Hand

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-15 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/#review17460 --- First round of my review, I haven't peek actual Nepomuk classes

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-15 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 15, 2012, 4:39 p.m.) Review request for Amarok, Vishesh Hand

Re: Review Request: GSoC : Nepomuk plugin for Amarok

2012-08-15 Thread Phalgun Guduthur
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106042/ --- (Updated Aug. 15, 2012, 4:38 p.m.) Review request for Amarok, Edward Hades