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

> On Авг. 6, 2015, 4:37 п.п., Pali Rohár wrote:
> > plugins/history/databaseconstants.h, line 1
> > <https://git.reviewboard.kde.org/r/124643/diff/1/?file=390725#file390725line1>
> >
> >     Just remove this whole file and rewrite code to better. Function <name> which just return <name> is useless. Column names are their meanings are already approved/reviewd and are not expected to be changed. In future for some unexpected reasons it will be needed, it can be be fixed...

I agree string constants must be stored in constants, not hard-coded into an application logic. Still, using static functions is excessive for such purpose, I'd suggest using static constants with .h-only initialization avoiding an extra compilation unit.


- Roman


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


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
[email protected]
https://mail.kde.org/mailman/listinfo/kopete-devel