Re: Review Request 124643: First review request for the history plugin

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



plugins/history/chathistoryplugin.cpp (lines 22 - 23)
<https://git.reviewboard.kde.org/r/124643/#comment58375>

    Two points:
    1) It's a constructor 'this' leakage, in case of exception mInstance will hold reference to deleted object. Please, avoid such a dangerous code.
    2) The intention is unclear. If this object is a singleton (what I'd advise you against), then second instance construction must be prohibited (it's not). mInstance should not be presented otherwise.



plugins/history/historybrowser.cpp (lines 40 - 41)
<https://git.reviewboard.kde.org/r/124643/#comment58376>

    It's not what we meant exactly. Something like public static const int WINDOW_MARGIN_SIZE should be added to the HistoryBrowser instead.



plugins/history/historybrowser.cpp (line 56)
<https://git.reviewboard.kde.org/r/124643/#comment58377>

    Looks all right now.


- Roman Nazarenko


On Aug. 24, 2015, 11:08 a.m., Joseph Joshua wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124643/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 11:08 a.m.)
> 
> 
> 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
[email protected]
https://mail.kde.org/mailman/listinfo/kopete-devel