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

[Milian Wolff]

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


Hey Salamander, there are still some issues here. Also, I just now notice that you want to target KDevelop 4.7. I'd say lets not do it there, but only do it in kf5-based master of kdevelop. Can you fix the issues and forward-port this patch to master? Also, please add a unit test, afaik in master there are even more tests, so you should find some inspiration on how to do this.

I'd especially want to see a unit test to check that the backwards compatibility is not broken.


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

    this is not necessary, is it?



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

    Sergey is correct, you'll have to check for the key to be non-empty, and continue otherwise. I still don't get why the isNull check is required for the value.



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

    this is wrong, this must be moved up below the `auto defMap = ...` line.


- Milian Wolff


On Feb. 2, 2015, 10:11 p.m., Salamander Purake wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122388/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2015, 10:11 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
> -----
> 
>   languages/plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp bcdebe5 
> 
> 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/20150203/2edd80c6/attachment.html>