Review Request 121196: Custom Build System human readable defines and includes path.

[Milian Wolff]

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


this patch is missing a unit test for the new format. the existing unit tests already check the legacy format.


languages/plugins/custom-definesandincludes/settingsmanager.cpp
<https://git.reviewboard.kde.org/r/121196/#comment49462>

    here and below: style
    
        while (...) {
          ...
        }
        
        if (...) {
          ...
        }
    
    and so forth



languages/plugins/custom-definesandincludes/settingsmanager.cpp
<https://git.reviewboard.kde.org/r/121196/#comment49463>

    as I told you on IRC, please use a distinct KConfigGroup for the Defines and then just write each entry of the defines hash into that group.
    
        KConfigGroup defines(pathgrp.group(ConfigConstants::definesKey));
        for (auto it = path.defines.begin(); it != path.defines.end(); ++it) {
          defines.writeEntry(it.key(), it.value());
        }



languages/plugins/custom-definesandincludes/settingsmanager.cpp
<https://git.reviewboard.kde.org/r/121196/#comment49464>

    this also needs to be rewritten when you change the format (see above)


- Milian Wolff


On Nov. 21, 2014, 6:56 a.m., Salamander Purake wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121196/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2014, 6:56 a.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
> -----
> 
>   languages/plugins/custom-definesandincludes/settingsmanager.h 6906cdb 
>   languages/plugins/custom-definesandincludes/settingsmanager.cpp 8ef77d3 
> 
> Diff: https://git.reviewboard.kde.org/r/121196/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/20141121/63f8236c/attachment.html>