Review Request: Allow to pass multiple targets and build variables to the make builder
Milian Wolff
mail at milianw.de
Fri Apr 13 21:27:37 UTC 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104537/#review12409
-----------------------------------------------------------
I think this needs some more work, esp. the QMap makes me cringe
projectbuilders/makebuilder/imakebuilder.h
<http://git.reviewboard.kde.org/r/104537/#comment9723>
hm a QMap will implicitly sort the variables, that might not be desired. i.e. if you want to have
make FOO=bar ASDF=asdf
you will actually get
make ASDF=asdf FOO=bar
instead it might be better to just go for a QStringList of arguments, which would also allow stuff like
make FOO BAR
(i.e. not A=B but just A B)
projectbuilders/makebuilder/imakebuilder.h
<http://git.reviewboard.kde.org/r/104537/#comment9724>
both versions of these functions should get a doxygen comment, could you add that, describing the new functionality please?
projectbuilders/makebuilder/imakebuilder.h
<http://git.reviewboard.kde.org/r/104537/#comment9722>
please don't inline this (it's virtual anyways), put it into the .cpp file
projectbuilders/makebuilder/imakebuilder.h
<http://git.reviewboard.kde.org/r/104537/#comment9725>
a comment here would be awesome as well please
projectbuilders/makebuilder/makejob.cpp
<http://git.reviewboard.kde.org/r/104537/#comment9726>
if you really leave it at a qmap (see issue above), please use iterators instead of foreach:
it = m_variables.constBegin();
while(it != m_variables.constEnd()) {
// it.key() , it.value()
++it;
}
- Milian Wolff
On April 11, 2012, 2:13 a.m., Alexandre Courbot wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104537/
> -----------------------------------------------------------
>
> (Updated April 11, 2012, 2:13 a.m.)
>
>
> Review request for KDevelop.
>
>
> Description
> -------
>
> This change augments the IMakeBuilder interface and MakeBuilder class to let them support the following:
>
> 1) Let make be run with multiple targets to build in one run
> 2) Pass build variables as a QMap of (variable, value) pairs that are also passed to make's command line.
>
> E.g. this change now makes it possible for the make builder to perform make invokations that look like the following (example taken from an actual Linux kernel build):
>
> $ make ARCH=arm CROSS_COMPILE=/usr/bin/arm-elf- vmlinux modules
>
> API compatibility is not broken, but ABI is as the former virtual method of IMakeBuilder is now an inline function.
>
>
> Diffs
> -----
>
> projectbuilders/makebuilder/imakebuilder.h 56735425d78551883f109e942145eba2aa982687
> projectbuilders/makebuilder/makebuilder.h 34881c6eaee775b6b8b53959dfcf825732e806da
> projectbuilders/makebuilder/makebuilder.cpp 6c6905db30f469958f4a0048826febea29bad15a
> projectbuilders/makebuilder/makejob.h 19032fdf371da793d52b3457e5aa78a6b8458150
> projectbuilders/makebuilder/makejob.cpp ad5636dbfdadf3ae18ad1cc5b8dff078dd34cd42
>
> Diff: http://git.reviewboard.kde.org/r/104537/diff/
>
>
> Testing
> -------
>
> Ensured API remained compatible and prior API behaved identically, tested build variables with the kdev-kernel plugin that uses them.
>
>
> Thanks,
>
> Alexandre Courbot
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120413/74b8b32d/attachment.html>
More information about the KDevelop-devel
mailing list