Review Request 122388: Custom Build System human readable defines and include search path for KDevelop kf5/master

Sergey Kalinichev kalinichev.so.0 at gmail.com
Mon Mar 2 07:39:50 UTC 2015


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



languages/plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp
<https://git.reviewboard.kde.org/r/122388/#comment52887>

    Here and below remove the comment, it's non informative. Also it's called unnamed/anonymous namespace not an empty one: http://stackoverflow.com/questions/154469/unnamed-anonymous-namespaces-vs-static-functions



languages/plugins/custom-definesandincludes/compilerprovider/tests/test_compilerprovider.h
<https://git.reviewboard.kde.org/r/122388/#comment52888>

    You don't need it here, remove it.



languages/plugins/custom-definesandincludes/compilerprovider/tests/test_compilerprovider.cpp
<https://git.reviewboard.kde.org/r/122388/#comment52890>

    Here and below, please, use this style instead: ...(SettingsManager& settings, KConfig& config)
    
    Also you should pass the config by a pointer.



projectmanagers/custom-buildsystem/kcm_kdevcustombuildsystem.desktop.cmake
<https://git.reviewboard.kde.org/r/122388/#comment52889>

    This is unrelated, remove it (probably you didn't rebase your patch before submitting)


There are still some issues. Please fix them, then we can commit it, I think.

- Sergey Kalinichev


On Feb. 27, 2015, 9:21 p.m., Salamander Purake wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122388/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:21 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> Made the defines and include path lists human readable by removing the QDataStream and QByte types from the save load steps.
> 
> 
> Diffs
> -----
> 
>   projectmanagers/custom-buildsystem/kcm_kdevcustombuildsystem.desktop.cmake d0e9e8f 
>   languages/plugins/custom-definesandincludes/compilerprovider/tests/test_compilerprovider.cpp f9fb7fe 
>   languages/plugins/custom-definesandincludes/compilerprovider/tests/test_compilerprovider.h e92ebc0 
>   languages/plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp df2c425 
> 
> Diff: https://git.reviewboard.kde.org/r/122388/diff/
> 
> 
> Testing
> -------
> 
> create/load a custom build system project (${project-name}.kdev4) add defines and includes path to project via in the text file and the dialog. Tested the defines lines inside the source code files and the autocompletion also in the code files.
> 
> 
> Thanks,
> 
> Salamander Purake
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150302/fc51a863/attachment.html>


More information about the KDevelop-devel mailing list