- User Since
- Jun 12 2015, 1:04 PM (192 w, 6 d)
Tue, Feb 19
Fri, Feb 8
Tue, Feb 5
Commit message ideally. Truncate the report where applicable.
Fri, Jan 25
Jan 14 2019
Jan 12 2019
Jan 11 2019
Jan 9 2019
Could you give me your full name + email for attribution in the commit?
I think that looks fine. Didn't test though.
Looks good to me. Can you push to 5.3 branch yourself?
@apol Something for you.
5.3 only please. Someone else (or even you) can merge it into master afterwards.
Jan 8 2019
Fixed with dcd5886fd949042765b905b3c67f9f20b18fb028
Jan 7 2019
Jan 6 2019
Dec 20 2018
Dec 15 2018
FYI: Patch looks good to me now, but I cannot judge whether it fixed the crash. So cannot give a +1.
Dec 14 2018
Dec 13 2018
Dec 11 2018
Dec 10 2018
Yep. please go for it.
@pino I actually thought about the same, but was unsure since Aleix asked to have it translated. I agree with your explanation, it makes sense to have this not translated at all. Will revert.
Dec 7 2018
Dec 3 2018
Just my 2 cent when I saw this review, didn't look further :)
Running into a crash with that patch, need to revisit:
I think this is a great addition and I see myself using this a lot. So far I've done these things through a set of .cpp files lying around in my $HOME and compiled/ran them using some command-line alias :)
LGTM in general. If you prefer your version (instead of the one proposed by me) feel free to push directly.
Not entirely sure about whether this is the right fix for this. Can someone check why this was commented before?
Okay, trying to keep this short.
Dec 2 2018
Just some general remarks: Storing the application's install prefix inside the binary itself almost always smells like bad design. Thus the concerns here. You rather want to do this dependent on the kdevelop binary's application path (cf. http://doc.qt.io/qt-5/qcoreapplication.html#applicationDirPath)
I've tested the idea of NOT specifying an install prefix default when none has been configured. To the best I can see this does not lead to invalid cmake invocations (aka -DCMAKE_INSTALL_PREFIX=). Instead, this does exactly what you'd expect: cmake will use whatever is its default for the current platform. The result will be obtained from the CMakeCache later on.
In other words, there doesn't appear to be any reason to hardcode assumptions about CMake's defaults, and not doing that should be a suitable compromise for everyone. It also removes the need for conditional code.
Forgot: Also needs documentation *in* source code why were are doing this. Noone can figure out that the temp dir is changed due to issues with the Clang backend here...
Not usable as-is.
Nov 30 2018
Nov 28 2018
Nov 27 2018
Nov 26 2018
Okay, but you could have pushed this directly ;)