- User Since
- Jun 12 2015, 1:04 PM (174 w, 3 d)
Nice find! Will cherry-pick to 5.3 branch.
Sorry, but this is yet another unmergeable WIP patch of yours. Just a quick review, I'm not diving into your code (which is difficult to parse, esp. the changes in CorePrivate::initialize...) right now.
Fri, Oct 12
@Petross404 Hmm, true. Let me push my patch anyway, for now.
Thu, Oct 11
Check whether base is defined at another place
Note: This is alternative approach to https://phabricator.kde.org/D15976
Hey @Petross404 -- the other day I reworked that script a bit, here's the result: https://phabricator.kde.org/D16123 -- I refactored it this way before seeing the new revision of yours. I do think my version is a little bit easier to grasp, since it does not have that many branches compared to yours.
I'm torn about this review; I'm with Aleix in the sense that this adds lots and lots of extra CMake code to maintain without a /lot/ of gain.
Tue, Oct 9
Can be committed this way as well, but I'd research using QScopedPoiner in some places instead.
Feel free to push after fixing the last remark.
ecm_mark_nongui_executable(kitengen) in CMake code has the same effect I think (and is the cleaner version)
Just hijacking this Diff b/c I see several places where this can be improved :)
Mon, Oct 8
Ah, right, I missed that part. The problem is that VS2017 by default doesn't set VS150COMNTOOLS as system-wide env variable.
Looks good to me, please push to 5.3 branch.
Pushed this for you. To 5.3 branch.
Sun, Oct 7
Looks good to me in general, feel free to push to master.
I don't see why not.
Fri, Oct 5
You're right. For the Windows case, we wouldn't actually need to query the Clang version at runtime, since we control the version of Clang/LLVM ourselves.
Thu, Oct 4
Otoh, Clang 7 "fixes" this since they no longer use "X.Y.Z/include" but just "X/include" -- so we do not have to worry about this any longer anyway.
Note: Doing the retrieval at runtime would also allow distro packages to upgrade the libclang version under-the-hood; without having the need to recompile KDevelop. I.e. going from Clang/LLVM 6.0.0 to 6.0.1 (and keeping ABI), KDevelop would keep working just fine (which it currently wouldn't...).