[Kde-pim] Review Request 112635: GID migration.

Fri Sep 13 23:06:11 BST 2013

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


One last round of pedantic coding style review :) 


migration/gid/gidmigrationjob.h
<http://git.reviewboard.kde.org/r/112635/#comment29527>

    *parent


migration/gid/gidmigrationjob.h
<http://git.reviewboard.kde.org/r/112635/#comment29528>

    *job


migration/gid/gidmigrationjob.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29529>

    *parent


migration/gid/gidmigrationjob.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29546>

    Space between foreach and "(", it's like "if (...)"


migration/gid/gidmigrationjob.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29530>

    *job


migration/gid/gidmigrator.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29531>

    spaces around "+"


migration/gid/gidmigrator.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29532>

    Add context for translators (i18nc)


migration/gid/gidmigrator.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29533>

    space after comma, not before :)


migration/gid/main.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29534>

    No spaces inside ()


migration/gid/main.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29535>

    No spaces inside ()


migration/gid/main.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29536>

    Maybe you could add 
    
    Akonadi::Control::widgetNeedsAkonadi(infoDialog);
    
    so that the dialog is disabled when Akonadi has to be started by start() above.


migration/migratorbase.h
<http://git.reviewboard.kde.org/r/112635/#comment29572>

    { on a new line


migration/migratorbase.h
<http://git.reviewboard.kde.org/r/112635/#comment29573>

    { on a new line


migration/migratorbase.h
<http://git.reviewboard.kde.org/r/112635/#comment29522>

    *parent


migration/migratorbase.h
<http://git.reviewboard.kde.org/r/112635/#comment29523>

    *parent


migration/migratorbase.h
<http://git.reviewboard.kde.org/r/112635/#comment29566>

    Could be const?


migration/migratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29521>

    &identifier, *parent


migration/migratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29524>

    & and * at the variable


migration/migratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29525>

    &msg


migration/migratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29526>

    i18nc("@info:status", "Not started");
    
    applies for the strings below too


migration/tests/testgidmigration.h
<http://git.reviewboard.kde.org/r/112635/#comment29537>

    *parent


migration/tests/testgidmigration.h
<http://git.reviewboard.kde.org/r/112635/#comment29538>

    Remove the spaces inside () here and everywhere below


migration/tests/testgidmigration.h
<http://git.reviewboard.kde.org/r/112635/#comment29539>

    Move & to the variable name


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29540>

    Remove spaces inside () here and everywhere below, move & to the variable name


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29541>

    Add {} please


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29542>

    & position


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29543>

    & position, spaces, { one a new line


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29544>

    No space after (, { one a new line


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29545>

    Space before "(", not after


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29547>

    *parent


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29548>

    I think you don't need to start up Akonadi manually when running in testrunner - the testrunner will spawn a unique Akonadi instance for you


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29549>

    No spaces


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29551>

    No spaces, QLatin1String()


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29552>

    Spaces


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29553>

    Spaces


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29554>

    Spaces


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29555>

    Spaces


migration/tests/testgidmigration.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29556>

    Spaces


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29557>

    & and * position


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29558>

    spaces around "+"


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29560>

    &identifier, *parent


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29561>

    Spaces around "+"


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29577>

    { one a new line (and everywhere below this)


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29563>

    Add a space between ) and {


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29567>

    QLatin1String()


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29574>

    { on a new line


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29568>

    QLatin1String()


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29576>

    { on a new line


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29569>

    Spaces around "+", QLatin1String


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29570>

    QLatin1String


migration/tests/testmigratorbase.cpp
<http://git.reviewboard.kde.org/r/112635/#comment29571>

    QLatin1String()


- Dan Vrátil


On Sept. 11, 2013, 12:55 p.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112635/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2013, 12:55 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> GID migration.
> 
> A new MigratorBase as interface for migrators.
> 
> I didn't adapt the existing KMigratorBase as it taylored towards one migrator per resource and I found a couple of other incompatibilities so I figured I rather make something clean from scratch.
> 
> It should be easy enough to adapt KMigratorBase to be based on top MigratorBase if we still need that code.
> 
> 
> Diffs
> -----
> 
>   migration/CMakeLists.txt 3d15b97ab49a092e56416dce1c88725bcca3695e 
>   migration/gid/CMakeLists.txt PRE-CREATION 
>   migration/gid/gidmigrationjob.h PRE-CREATION 
>   migration/gid/gidmigrationjob.cpp PRE-CREATION 
>   migration/gid/gidmigrator.h PRE-CREATION 
>   migration/gid/gidmigrator.cpp PRE-CREATION 
>   migration/gid/main.cpp PRE-CREATION 
>   migration/infodialog.h 0003c48d3eab4260e4f0cc6a3ebf17ac762a1e1f 
>   migration/infodialog.cpp 6f08db23922c181b98d809d77b98564f7b6a3ca8 
>   migration/kmigratorbase.h 3a57917f0d0655320147ac13ef9544975c0188d0 
>   migration/migratorbase.h PRE-CREATION 
>   migration/migratorbase.cpp PRE-CREATION 
>   migration/tests/CMakeLists.txt a9129a6bd033867345a668c2fdda0c3c0adae7d6 
>   migration/tests/testgidmigration.h PRE-CREATION 
>   migration/tests/testgidmigration.cpp PRE-CREATION 
>   migration/tests/testmigratorbase.cpp PRE-CREATION 
>   migration/tests/unittestenv/config.xml fa0eafe88d573518ad72269f1b8aafb7f93c885a 
>   migration/tests/unittestenv/kdehome/share/config/akonadi_knut_resource_0rc PRE-CREATION 
>   migration/tests/unittestenv/kdehome/share/config/kdebugrc 32317f7453f4c0a0ec778bd494ced8e7d65078f2 
>   migration/tests/unittestenv/kdehome/testdata-res1.xml PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112635/diff/
> 
> 
> Testing
> -------
> 
> 
> 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/