Review Request 119057: Split the IncludePathResolver into MakeFileResolver and NoProjectIncludePathsManager

Milian Wolff mail at milianw.de
Wed Jul 9 13:33:28 UTC 2014


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

Ship it!


Looks good to me. The style stuff I mention below is probably not your new code, but all issues from the old ugly mess ;-) Maybe you could fix a few things here and there while at it though.

Thanks!


languages/cpp/codegen/unresolvedincludeassistant.h
<https://git.reviewboard.kde.org/r/119057/#comment43141>

    while at it, make it const&



languages/cpp/makefileresolver.h
<https://git.reviewboard.kde.org/r/119057/#comment43137>

    here and below, please don't inline this stuff



languages/cpp/makefileresolver.h
<https://git.reviewboard.kde.org/r/119057/#comment43138>

    make this function const?



languages/cpp/makefileresolver.h
<https://git.reviewboard.kde.org/r/119057/#comment43140>

    const?



languages/cpp/makefileresolver.h
<https://git.reviewboard.kde.org/r/119057/#comment43139>

    const?


- Milian Wolff


On July 1, 2014, 11:55 a.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119057/
> -----------------------------------------------------------
> 
> (Updated July 1, 2014, 11:55 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> This is needed as it will let us to get rid of the .kdev_include_paths related code from the kdev-clang. Also it'll make it easier to extract the MakeFileResolver in the future.
> 
> Also simplified the code.
> 
> 
> Diffs
> -----
> 
>   languages/cpp/CMakeLists.txt 48f12da 
>   languages/cpp/codegen/customincludepaths.h 1041682 
>   languages/cpp/codegen/customincludepaths.cpp 8e4e41e 
>   languages/cpp/codegen/ui/custom_include_paths.ui 37be9b5 
>   languages/cpp/codegen/unresolvedincludeassistant.h 60a048b 
>   languages/cpp/codegen/unresolvedincludeassistant.cpp 7c3bc49 
>   languages/cpp/cpplanguagesupport.h e1659ae 
>   languages/cpp/cpplanguagesupport.cpp 79530e9 
>   languages/cpp/includepathcomputer.h 61de3ec 
>   languages/cpp/includepathcomputer.cpp 61c3646 
>   languages/cpp/includepathresolver.h c9e9d7d 
>   languages/cpp/includepathresolver.cpp 3cffeb1 
>   languages/cpp/makefileresolver.h PRE-CREATION 
>   languages/cpp/makefileresolver.cpp PRE-CREATION 
>   languages/cpp/tests/CMakeLists.txt 68ba233 
>   languages/plugins/custom-definesandincludes/CMakeLists.txt 4dc013c 
>   languages/plugins/custom-definesandincludes/definesandincludesmanager.h f996a83 
>   languages/plugins/custom-definesandincludes/definesandincludesmanager.cpp 98f32fc 
>   languages/plugins/custom-definesandincludes/noprojectincludesanddefines/CMakeLists.txt PRE-CREATION 
>   languages/plugins/custom-definesandincludes/noprojectincludesanddefines/noprojectcustomincludepaths.h PRE-CREATION 
>   languages/plugins/custom-definesandincludes/noprojectincludesanddefines/noprojectcustomincludepaths.cpp PRE-CREATION 
>   languages/plugins/custom-definesandincludes/noprojectincludesanddefines/noprojectcustomincludepaths.ui PRE-CREATION 
>   languages/plugins/custom-definesandincludes/noprojectincludesanddefines/noprojectincludepathsmanager.h PRE-CREATION 
>   languages/plugins/custom-definesandincludes/noprojectincludesanddefines/noprojectincludepathsmanager.cpp PRE-CREATION 
>   languages/plugins/custom-definesandincludes/tests/definesandincludestest.h 1589b78 
>   languages/plugins/custom-definesandincludes/tests/definesandincludestest.cpp 9c421bb 
>   languages/plugins/custom-definesandincludes/tests/projectsgenerator.h b919ae5 
>   languages/plugins/custom-definesandincludes/tests/projectsgenerator.cpp 5d08a25 
> 
> Diff: https://git.reviewboard.kde.org/r/119057/diff/
> 
> 
> Testing
> -------
> 
> Added an unit test for NoProjectIncludePathsManager.
> 
> 
> File Attachments
> ----------------
> 
> kdevplatform.diff
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/07/01/39f48afe-f63e-46a1-ba1f-12a7a0d48a98__kdevplatform.diff
> before.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/07/01/bf114c23-763e-47b8-a153-70c57b14ecf7__before.png
> after.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/07/01/b5cdd3d1-6b5e-4b6a-8075-21f8c9655f8b__after.png
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

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


More information about the KDevelop-devel mailing list