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

[Dan Vrátil]

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


So far looks good :)

One question though: does it make sense to hardcode "migrationagent" in ignored sessions in Akonadi::Monitor so that everyone ignores changes made by this agent? Otherwise during the migration we will be spamming other agents and clients with notifications about item changes even though they don't need to know about it at all.


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

    Wouldn't show() be better to avoid nested event loop when dispatching migration subjobs?



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

    The convention for naming getters in Qt/KDE is to usually omit the "get" word



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

    QString::fromLatin1("%1 %%").arg(migration->progress())



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

    You should make sure the migrator is not already to prevent accidentally starting it twice.



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

    Wouldn't just KTextView (or smthing like that) better than a listview? In case of a bug we will want users to copy content of the log to bugzilla, and it's hard to copy stuff from a listview :)



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

    QLatin1String()



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

    QLatin1String()


- Dan Vrátil


On Sept. 10, 2013, 3:17 p.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112636/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2013, 3:17 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/CMakeLists.txt 22b6db73687f82c9d05a19c63708be32b6508513 
>   agents/migration/CMakeLists.txt PRE-CREATION 
>   agents/migration/migrationagent.cpp PRE-CREATION 
>   agents/migration/migrationagent.desktop PRE-CREATION 
>   agents/migration/migrationagent.h PRE-CREATION 
>   agents/migration/migrationscheduler.h PRE-CREATION 
>   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/