Review Request: GSoC : Nepomuk plugin for Amarok

Matěj Laitl matej at laitl.cz
Fri Sep 14 09:39:35 UTC 2012 


> On Sept. 13, 2012, 7:43 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, lines 151-154
> > <http://git.reviewboard.kde.org/r/106042/diff/8/?file=85305#file85305line151>
> >
> >     What about my comments about moving this above the main query?
> 
> Phalgun Guduthur wrote:
>     I forgot talking about it in my previous comments, the reason for this is that I don't know the number of tracks that will be parsed by the query ( total tracks ). I run a separate query just to find out the number of tracks. Since that value is calculated only here, I had to move the logger initiation to the job itself.

I understand, I just thought you can call newProgressOperation() right after you known totalTracks, whis is about a hundred lines above.


> On Sept. 13, 2012, 7:43 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp, lines 161-163
> > <http://git.reviewboard.kde.org/r/106042/diff/8/?file=85305#file85305line161>
> >
> >     The explanation is rather strange. You said earlier that the ontologies are defined as doubles. If it is the case the right explanation would be that you need to use QString::toDouble() to be able to parse the real number stored as int.
> 
> Phalgun Guduthur wrote:
>     >>>> it was leading to erraneous values due to the size limitation of int.
>     
>     --> this was because the ontologies had the range of double. 
>     
>     But the documentation doesn't say it, I'll fix that.

Okay. I still think this is not about *range*, but about *string representation*. Int has range of ?2,147,483,648 to 2,147,483,647.


> On Sept. 13, 2012, 7:43 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/nepomukcollection/NepomukCollection.cpp, line 79
> > <http://git.reviewboard.kde.org/r/106042/diff/8/?file=85299#file85299line79>
> >
> >     This has no effect as no one has pointer to Collection is its constructor. It is just confusing and should be removed.
> >     
> >     It is the trird time I'm saying this.
> 
> Phalgun Guduthur wrote:
>     Oops, missed this one. 
>     
>     Sorry about that strohel, I'll fix this one. I thank you for your patience through my silly mistakes. :-)

:-)


- Matěj


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106042/#review18939
-----------------------------------------------------------


On Sept. 13, 2012, 6:58 p.m., Phalgun Guduthur wrote:
> 
> -----------------------------------------------------------
> 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 Hades Toroshchin, Vishesh Handa, and Matěj Laitl.
> 
> 
> Description
> -------
> 
> Nepomuk plugin for Amarok.
> 
> Almost all of the code changes can be found in src/core-impl/collections/nepomukcollection/*
> 
> Code builds and after Nepomuk plugin is activated in the "Settings" dialog, Nepomuk Plugin comes into play and queries all the tracks in your machine. The query is not 'that' fast and might take several seconds depending on the number of tracks in your box. 
> IMPORTANT : Make sure Nepomuk is enabled if you want to give the plugin a spin. 
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/CMakeLists.txt c78b9202ece71b51189c4e47d85acfa4a74ef8d6 
>   src/core-impl/collections/nepomukcollection/CMakeLists.txt 7cfd4b056000cf5de18c87d1d014b6670703e796 
>   src/core-impl/collections/nepomukcollection/NepomukAlbum.h 185c25a0fe5b19248a3ab40c1d9d84fd66e6d2fe 
>   src/core-impl/collections/nepomukcollection/NepomukAlbum.cpp 6a09a1bbb4ea9bdfc08280326d29a351c666ab25 
>   src/core-impl/collections/nepomukcollection/NepomukArtist.h 6fcedf3ac3724083b6992deb71fb659d9b2dc5d0 
>   src/core-impl/collections/nepomukcollection/NepomukArtist.cpp 13ddf0142796d90af265d28a06d60110da64f138 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.h 928b1458782f0145a012c81468f22edfafc0f547 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.cpp cb185e818de2e00091f9cb03f4b19ccface14635 
>   src/core-impl/collections/nepomukcollection/NepomukCollectionFactory.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukCollectionFactory.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukComposer.h 1b11325ec488f202a7b13b10d36c8216b487ae89 
>   src/core-impl/collections/nepomukcollection/NepomukComposer.cpp f21251eab6798bb499d01900151b2c9a1783deae 
>   src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukGenre.h ce0e3b71515d88e57dd3d01beba85e3cdfd8ede6 
>   src/core-impl/collections/nepomukcollection/NepomukGenre.cpp 945074c4737ac2856469d5041ca2ea888d609bad 
>   src/core-impl/collections/nepomukcollection/NepomukQueryMaker.h 50067decec72f34a845e1da50e74cdf19e9c0f83 
>   src/core-impl/collections/nepomukcollection/NepomukQueryMaker.cpp 33163eaa0b279dedcf92de01346312930f10d944 
>   src/core-impl/collections/nepomukcollection/NepomukRegistry.h a21347eca2ab519a3c8b5b1f14650878fd7b4333 
>   src/core-impl/collections/nepomukcollection/NepomukRegistry.cpp 8afa199f73035eb7d95a8913eb1cbe9fea8b2ebd 
>   src/core-impl/collections/nepomukcollection/NepomukTrack.h 77dd8c70c8b0727655dfe1db89c7bd19208e77e5 
>   src/core-impl/collections/nepomukcollection/NepomukTrack.cpp 7db01cf34b3765f18f8b8b3cf6efbdf07af6e564 
>   src/core-impl/collections/nepomukcollection/NepomukYear.h 504cbe2b146ae9a53291de9e82fa384467eb14e1 
>   src/core-impl/collections/nepomukcollection/NepomukYear.cpp 1f13de0bd24e56b1b64b8c45f1d22720dd487a3c 
>   src/core-impl/collections/nepomukcollection/amarok_collection-nepomukcollection.desktop 815e69e492e819740aba620cc399a8ee79eace74 
>   src/core-impl/collections/nepomukcollection/meta/NepomukAlbum.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukAlbum.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukArtist.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukArtist.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukComposer.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukComposer.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukGenre.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukGenre.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukLabel.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukLabel.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukYear.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukYear.cpp PRE-CREATION 
>   src/core-impl/collections/support/MemoryMeta.cpp 37ba510f61605af7ebd803aee3529bde18ad84c5 
> 
> Diff: http://git.reviewboard.kde.org/r/106042/diff/
> 
> 
> Testing
> -------
> 
> Minimal. Plan to spend the remaining time on testing the plugin. 
> 
> 
> Thanks,
> 
> Phalgun Guduthur
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120914/15a854d7/attachment-0001.html>

More information about the Amarok-devel mailing list