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



plugins/history/databasemanager.cpp (line 55)
<https://git.reviewboard.kde.org/r/124643/#comment57806>

    Consider reporting a more exact error message (including DB driver reply). 
It's generally helpful knowing if either password is invalid or your network 
card is a bubble.



plugins/history/databasemanager.cpp (line 67)
<https://git.reviewboard.kde.org/r/124643/#comment57805>

    You emit 'Database not open' in case of opening failure in initDatabase and 
'Database not open.' in case of insertMessage failure. Consider writing more 
specific messages (possibly with some extra issue-specific info).



plugins/history/databasemanager.cpp (lines 97 - 102)
<https://git.reviewboard.kde.org/r/124643/#comment57800>

    Consider using same formatting all along these lines for readablility 
reasons.



plugins/history/databasemanager.cpp (lines 119 - 123)
<https://git.reviewboard.kde.org/r/124643/#comment57801>

    To the external function.



plugins/history/databasemanager.cpp (lines 120 - 122)
<https://git.reviewboard.kde.org/r/124643/#comment57802>

    How will you search among non-sorted ids? Are 1,2,3 and 2,3,1 considered 
being different chats?



plugins/history/databasemanager.cpp (lines 121 - 125)
<https://git.reviewboard.kde.org/r/124643/#comment57803>

    Code duplication. Code duplication is all around us!



plugins/history/databasemanager.cpp (line 136)
<https://git.reviewboard.kde.org/r/124643/#comment57804>

    Query failure is silently ignored.



plugins/history/databasemanager.cpp (line 144)
<https://git.reviewboard.kde.org/r/124643/#comment57807>

    This is an error if Kopete Plugins can be disabled in runtime (i.e. their 
lifecycle is not bound to a kapp). Consider consulting Pali whether such 
behaviour is possible.


- Roman Nazarenko


On Авг. 6, 2015, 12:10 п.п., Joseph Joshua wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124643/
> -----------------------------------------------------------
> 
> (Updated Авг. 6, 2015, 12:10 п.п.)
> 
> 
> Review request for Kopete and Pali Rohár.
> 
> 
> Repository: kopete
> 
> 
> Description
> -------
> 
> GSoC 2015: History Plugin
> 
> 
> Diffs
> -----
> 
>   plugins/CMakeLists.txt 86fe572 
>   plugins/history/CMakeLists.txt 015df95 
>   plugins/history/chathistoryplugin.h PRE-CREATION 
>   plugins/history/chathistoryplugin.cpp PRE-CREATION 
>   plugins/history/converter.cpp dd56014 
>   plugins/history/databaseconstants.h PRE-CREATION 
>   plugins/history/databaseconstants.cpp PRE-CREATION 
>   plugins/history/databasemanager.h PRE-CREATION 
>   plugins/history/databasemanager.cpp PRE-CREATION 
>   plugins/history/historybrowser.h PRE-CREATION 
>   plugins/history/historybrowser.cpp PRE-CREATION 
>   plugins/history/historybrowser.ui PRE-CREATION 
>   plugins/history/historychatui.rc 6e2c072 
>   plugins/history/historyconfig.kcfg 0606910 
>   plugins/history/historydialog.h 5ba20ce 
>   plugins/history/historydialog.cpp 0720cb7 
>   plugins/history/historyguiclient.h 35a32ab 
>   plugins/history/historyguiclient.cpp 08b5816 
>   plugins/history/historyimport.h a922df7 
>   plugins/history/historyimport.cpp 9d9aa15 
>   plugins/history/historylogger.h e032f2f 
>   plugins/history/historylogger.cpp 8a34c85 
>   plugins/history/historyplugin.h 35cc4f9 
>   plugins/history/historyplugin.cpp bedd560 
>   plugins/history/historypreferences.h e74a3b7 
>   plugins/history/historypreferences.cpp c10005c 
>   plugins/history/historypreferencesui.ui PRE-CREATION 
>   plugins/history/historyprefsui.ui 6cd3b87 
>   plugins/history/historyui.rc 5f72b22 
>   plugins/history/historyviewer.ui e730b20 
>   plugins/history/kopete_history.desktop b96742a 
>   plugins/history/kopete_history_config.desktop 337e943 
>   plugins/history2/CMakeLists.txt 195edf6 
>   plugins/history2/history2chatui.rc 6e2c072 
>   plugins/history2/history2config.kcfg 17055b6 
>   plugins/history2/history2config.kcfgc e87f8ca 
>   plugins/history2/history2dialog.h 29b1b5c 
>   plugins/history2/history2dialog.cpp e5fa8a2 
>   plugins/history2/history2guiclient.h d07ce18 
>   plugins/history2/history2guiclient.cpp 6a4c4c5 
>   plugins/history2/history2import.h c2760f6 
>   plugins/history2/history2import.cpp db103fe 
>   plugins/history2/history2logger.h 13f159a 
>   plugins/history2/history2logger.cpp cad59df 
>   plugins/history2/history2plugin.h 424861a 
>   plugins/history2/history2plugin.cpp ca4d580 
>   plugins/history2/history2preferences.h 2782254 
>   plugins/history2/history2preferences.cpp 59dfb8c 
>   plugins/history2/history2prefsui.ui b2ffab2 
>   plugins/history2/history2ui.rc 5f72b22 
>   plugins/history2/history2viewer.ui eb670fe 
>   plugins/history2/kopete_history2.desktop 7bee952 
>   plugins/history2/kopete_history2_config.desktop 7c39892 
> 
> Diff: https://git.reviewboard.kde.org/r/124643/diff/
> 
> 
> Testing
> -------
> 
> -Builds successfully
> -Chats are logged into the database
> -Chat history is appended to new chats
> -Chat history can be browsed
> 
> 
> Thanks,
> 
> Joseph Joshua
> 
>

_______________________________________________
kopete-devel mailing list
kopete-devel@kde.org
https://mail.kde.org/mailman/listinfo/kopete-devel

Reply via email to