[Kde-pim] Review Request 124979: Gracefully quit on configuration/database startup errors

[Daniel Vrátil]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124979/#review84781
-----------------------------------------------------------


Sorry for taking so long with this review. The patch looks good to me, there are some coding style nitpicks below, so please fix them, but overall it seems OK.


src/server/akonadi.cpp (line 312)
<https://git.reviewboard.kde.org/r/124979/#comment58672>

    add {}



src/server/search/searchtaskmanagerthread.cpp (line 42)
<https://git.reviewboard.kde.org/r/124979/#comment58673>

    Add {}



src/server/storage/datastore.cpp (line 214)
<https://git.reviewboard.kde.org/r/124979/#comment58674>

    return sInstances.hasLocalData();



src/server/storage/dbconfig.cpp (line 64)
<https://git.reviewboard.kde.org/r/124979/#comment58675>

    return s_DbConfigInstance;



src/server/storage/dbconfig.cpp (line 99)
<https://git.reviewboard.kde.org/r/124979/#comment58676>

    Move { on the line above



src/server/storage/dbconfigsqlite.cpp (line 90)
<https://git.reviewboard.kde.org/r/124979/#comment58677>

    Missing {}


- Daniel Vrátil


On Aug. 29, 2015, 5:51 a.m., Dāvis Mosāns wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124979/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2015, 5:51 a.m.)
> 
> 
> Review request for Akonadi.
> 
> 
> Repository: akonadi
> 
> 
> Description
> -------
> 
> Gracefully quit on configuration/database startup errors.
> When database haven't been started or fails to start then Akonadi would crash itself with akFatal and produce backtrace. Also akonadicontrol would try to restart but it's pointless for this case.
> It's much better for these cases to simply output error and quit without crashing.
> I think akFatal should be be used only for cases when something happened which shouldn't happen but things outside of akonadi control (eg. database) should be handled gracfully as there's no point in backtrace anyway as it's not Akonadi bug.
> 
> 
> Diffs
> -----
> 
>   src/akonadicontrol/main.cpp 56ea58137803d9494290d6086767c09e94b29fd5 
>   src/akonadicontrol/processcontrol.cpp ae721cbc1f818ec2c093f0ca617faa862039fa1c 
>   src/server/akonadi.h f866e5808b94ffbbdab237ccacc083e6735101e9 
>   src/server/akonadi.cpp cb9ec513c8b5f8f65270c1eea6dda1edb163519a 
>   src/server/search/searchtaskmanagerthread.cpp 3843531afcab5d10845f34615b326e224d09391d 
>   src/server/storage/datastore.h 3edbfc93ec09f13b100958d7d0bfd2bcbaab687c 
>   src/server/storage/datastore.cpp b4bd974b2d6e6d4b7ad5e94f82bfc3618e14b2ac 
>   src/server/storage/dbconfig.h cf7b38bacfec2bf965ec81a2366ab1643e94eac7 
>   src/server/storage/dbconfig.cpp 095091f4f172f44e4c45d2132d251855639d1b69 
>   src/server/storage/dbconfigmysql.h 57020d01d7ece055ab72af0c122db210329e4278 
>   src/server/storage/dbconfigmysql.cpp dc4c8ab8bc1b0581e5b2794839c051688b916566 
>   src/server/storage/dbconfigpostgresql.h d75a907d3e50c23aa2bede1917af1ffa40969078 
>   src/server/storage/dbconfigpostgresql.cpp db387a989c5d67fca775324b5c5b3b39e5dc31f1 
>   src/server/storage/dbconfigsqlite.cpp 7682b779b6e02474c533d6e5799a339b88d231c4 
> 
> Diff: https://git.reviewboard.kde.org/r/124979/diff/
> 
> 
> Testing
> -------
> 
> Compiles and tested that it quits normallly if database haven't been started.
> 
> 
> Thanks,
> 
> Dāvis Mosāns
> 
>

_______________________________________________
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/