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

Matěj Laitl matej at laitl.cz
Thu Nov 14 20:12:10 UTC 2013


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

Ship it!


Looks good to me. You can either merge (fixing some trivial things below) right now or wait for me to review the rest of your GSoC.


src/statsyncing/Controller.h
<http://git.reviewboard.kde.org/r/113272/#comment31389>

    I think none of these includes are needed any more.



src/statsyncing/Provider.h
<http://git.reviewboard.kde.org/r/113272/#comment31390>

    Is this include needed?



src/statsyncing/ui/ConfigureProviderDialog.h
<http://git.reviewboard.kde.org/r/113272/#comment31392>

    We're not really consistent in indentation of namespaced class definitions in Amarok code, but please keep consistency within the statsyncing component: indent as in statsyncing/Track.h etc.



src/statsyncing/ui/CreateProviderDialog.h
<http://git.reviewboard.kde.org/r/113272/#comment31394>

    Please rename include guard to match class name



src/statsyncing/ui/CreateProviderDialog.h
<http://git.reviewboard.kde.org/r/113272/#comment31393>

    ditto indentation


- Matěj Laitl


On Nov. 6, 2013, 8:19 p.m., Konrad Zemek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113272/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 8:19 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Repository: amarok
> 
> 
> Description
> -------
> 
> These are changes in the StatSyncing framework that I made as a part of my project.
> 
> 
> Diffs
> -----
> 
>   src/statsyncing/Config.h 140e647 
>   src/statsyncing/Config.cpp dd82dbe 
>   src/statsyncing/Controller.h 10c72a8 
>   src/statsyncing/Controller.cpp bf4c5d8 
>   src/statsyncing/Provider.h d9663f9 
>   src/statsyncing/Provider.cpp 775fce3 
>   src/statsyncing/ProviderFactory.h PRE-CREATION 
>   src/statsyncing/ProviderFactory.cpp PRE-CREATION 
>   src/statsyncing/SimpleTrack.h PRE-CREATION 
>   src/statsyncing/SimpleTrack.cpp PRE-CREATION 
>   src/statsyncing/SimpleWritableTrack.h PRE-CREATION 
>   src/statsyncing/SimpleWritableTrack.cpp PRE-CREATION 
>   src/statsyncing/Track.h 2f91704 
>   src/statsyncing/Track.cpp 9655cc1 
>   src/statsyncing/TrackTuple.h 157d604 
>   src/statsyncing/TrackTuple.cpp 9442794 
>   src/statsyncing/jobs/SynchronizeTracksJob.cpp b789aa3 
>   src/statsyncing/ui/ConfigureProviderDialog.h PRE-CREATION 
>   src/statsyncing/ui/ConfigureProviderDialog.cpp PRE-CREATION 
>   src/statsyncing/ui/CreateProviderDialog.h PRE-CREATION 
>   src/statsyncing/ui/CreateProviderDialog.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/113272/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Konrad Zemek
> 
>

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


More information about the Amarok-devel mailing list