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