Review Request 119613: Use Makefile include path resolution only for CustomMake projects.

[Milian Wolff]

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

Ship it!


Yeah, that looks good! And if we ever need to use that resolver from a different project manager, we can always make the code shareable in the future. So +1 from my side, but please change the way you register background providers (see below).


languages/plugins/custom-definesandincludes/idefinesandincludesmanager.h
<https://git.reviewboard.kde.org/r/119613/#comment44512>

    I'd prefer if we could instead register a Provider as a background-provider. I.e. just add a registerBackgroundProvider/unregisterBackgroundProvider and put it into a separate list in the manager.
    
    This would make this class obsolete and no dynamic casts will be required when looking for stuff from the background


- Milian Wolff


On Aug. 5, 2014, 9:22 a.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119613/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 9:22 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> IMO it doesn't make sense at all to run "make" to compute include directories for projects that don't use Makefiles. Even if some project managers indirectly use Makefiles and can't compute include directories themselves, then it should be fixed there, and not workarounded by calling MakeFile include path resolver, which btw works only with very simple Makefiles, and as a result, at least for me, it almost never succeeds.
> 
> 
> Diffs
> -----
> 
>   languages/cpp/includepathcomputer.cpp 792dd71 
>   languages/plugins/custom-definesandincludes/CMakeLists.txt d1be9a1 
>   languages/plugins/custom-definesandincludes/definesandincludesmanager.h ac224dc 
>   languages/plugins/custom-definesandincludes/definesandincludesmanager.cpp fd7b095 
>   languages/plugins/custom-definesandincludes/idefinesandincludesmanager.h 3a6e87a 
>   languages/plugins/custom-definesandincludes/makefileresolver/CMakeLists.txt 6211f3a 
>   languages/plugins/custom-definesandincludes/makefileresolver/makefileresolver.h a493e62 
>   languages/plugins/custom-definesandincludes/makefileresolver/makefileresolver.cpp 628d37d 
>   languages/plugins/custom-definesandincludes/makefileresolver/tests/CMakeLists.txt d41f0ec 
>   languages/plugins/custom-definesandincludes/makefileresolver/tests/test_custommake.h 16c05f1 
>   languages/plugins/custom-definesandincludes/makefileresolver/tests/test_custommake.cpp 5a55262 
>   projectmanagers/custommake/CMakeLists.txt f80979f 
>   projectmanagers/custommake/custommakemanager.h e230d09 
>   projectmanagers/custommake/custommakemanager.cpp 2f6d2f2 
>   projectmanagers/custommake/kdevcustommakemanager.desktop.cmake aab7839 
>   projectmanagers/custommake/makefileresolver/CMakeLists.txt PRE-CREATION 
>   projectmanagers/custommake/makefileresolver/makefileresolver.h PRE-CREATION 
>   languages/cpp/includepathcomputer.h 70851a2 
>   projectmanagers/custommake/makefileresolver/makefileresolver.cpp PRE-CREATION 
>   projectmanagers/custommake/makefileresolver/tests/CMakeLists.txt PRE-CREATION 
>   projectmanagers/custommake/makefileresolver/tests/test_custommake.h PRE-CREATION 
>   projectmanagers/custommake/makefileresolver/tests/test_custommake.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119613/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

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