Review Request: TrackProvider non virtual functions implemented for Nepomuk Collection

Matěj Laitl matej at laitl.cz
Fri Nov 9 15:16:53 UTC 2012


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


The code looks good, altough I'm against copying documentation from the superclass. vHanda is certainly able to use his IDE to lookup the documentation for hip, and you'll struggle to keep it up to date.

The documentation for trackForUidUrl() is on the other hand welcome. There are whitespace errors like too much indentation in possiblyContainsTrack() and no newlines surrounding trackForUidUrl() declaratio in .h file. (I cannot comment inline, reviewboards shows an exception here)

- Matěj Laitl


On Nov. 9, 2012, 2:24 p.m., Phalgun Guduthur wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107161/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2012, 2:24 p.m.)
> 
> 
> Review request for Amarok, Edward Hades Toroshchin and Vishesh Handa.
> 
> 
> Description
> -------
> 
> First footstep in completing the Nepomuk Collection. 
> 
> The non virtual methods belonging to TrackProvider implemented.
> Repeated documentation to help vHanda understand the purpose of those functions. 
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.h d920ff6 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.cpp 0593697 
> 
> Diff: http://git.reviewboard.kde.org/r/107161/diff/
> 
> 
> Testing
> -------
> 
> Minimal.
> 
> 
> Thanks,
> 
> Phalgun Guduthur
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20121109/f6b0a7b0/attachment.html>


More information about the Amarok-devel mailing list