[Kde-pim] Review Request 112636: Migration agent that schedules various migrators and exposes an interface for status and control.

Dan Vrátil dvratil at redhat.com
Fri Sep 13 23:19:37 BST 2013


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


A little bit more of coding style pedantry


agents/migration/migrationagent.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29578>

    i18nc() - add context



agents/migration/migrationscheduler.h
<http://git.reviewboard.kde.org/r/112636/#comment29580>

    { on a new line



agents/migration/migrationscheduler.h
<http://git.reviewboard.kde.org/r/112636/#comment29579>

    No spaces inside ()



agents/migration/migrationscheduler.h
<http://git.reviewboard.kde.org/r/112636/#comment29581>

    { on a new line



agents/migration/migrationscheduler.h
<http://git.reviewboard.kde.org/r/112636/#comment29584>

    Why not just work with pointers here? You need to pass pointer to the listview anyway...



agents/migration/migrationstatuswidget.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29587>

    i18nc



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29589>

    put the MigratorBase... on a new line, add spaces around the "+" operator and put {} on a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29590>

    {, return... and } each on a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29591>

    {} ona a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29592>

    { on a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29593>

    { one a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29594>

    {, return ... and } each on a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29595>

    { on a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29596>

    { one a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29597>

    spaces after comma



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29598>

    space after comma



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29600>

    { on a new line



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29599>

    space after comma



agents/migration/tests/schedulertest.cpp
<http://git.reviewboard.kde.org/r/112636/#comment29601>

    { on a new line...fix also everywhere below


- Dan Vrátil


On Sept. 11, 2013, 12:41 p.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112636/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2013, 12:41 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> Migration agent that schedules various migrators and exposes an interface for status and control.
> 
> This does not yet include a plugin interface for making plugging in new migrators plugin based. That would be easy enough to add but only makes sense if we push the MigratorBase interface to kdepimlibs along with a dbus control interface, so i.e. applications such as kmail can install new migration plugins and control them via the dbus interface.
> 
> The agent automatically starts migration jobs that have shouldAutostart() enabled and exposes an interface in it's configuration dialog to start/stop migrators. The interface is yet very basic and is also no necessarily in the most convenient place. We should likely have some central place for controlling background processes or simply ensure the automatic processing is handled well enough.
> 
> Ideas how to do that best are appreciated.
> 
> 
> Diffs
> -----
> 
>   agents/migration/migrationscheduler.h PRE-CREATION 
>   agents/migration/migrationagent.h PRE-CREATION 
>   agents/migration/migrationagent.desktop PRE-CREATION 
>   agents/migration/migrationagent.cpp PRE-CREATION 
>   agents/migration/CMakeLists.txt PRE-CREATION 
>   agents/CMakeLists.txt 22b6db73687f82c9d05a19c63708be32b6508513 
>   agents/migration/migrationscheduler.cpp PRE-CREATION 
>   agents/migration/migrationstatuswidget.h PRE-CREATION 
>   agents/migration/migrationstatuswidget.cpp PRE-CREATION 
>   agents/migration/tests/CMakeLists.txt PRE-CREATION 
>   agents/migration/tests/schedulertest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112636/diff/
> 
> 
> Testing
> -------
> 
> I tried the GID migration for contacts, which worked fine for me.
> 
> 
> Thanks,
> 
> Christian Mollekopf
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


More information about the kde-pim mailing list