<div class="gmail_quote">On Tue, Aug 4, 2009 at 1:39 PM, Andreas Pakulat <span dir="ltr"><<a href="mailto:apaku@gmx.de">apaku@gmx.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On 04.08.09 12:57:42, Aleix Pol wrote:<br>
> On Tue, Aug 4, 2009 at 9:18 AM, Andreas Pakulat <<a href="mailto:apaku@gmx.de">apaku@gmx.de</a>> wrote:<br>
</div><div class="im">> > > > > +configure_file(<br>
> > ${CMAKE_CURRENT_SOURCE_DIR}/config-kdevplatform.h.cmake<br>
> > > > > +                ${CMAKE_CURRENT_BINARY_DIR}/config-kdevplatform.h )<br>
> > > > > +<br>
> > > ><br>
> > > > Even more code to execute.<br>
> > ><br>
> > > I wonder what you mean. Every single cmake project use configure file for<br>
> > > conditional compilation inside files. I don't think is that much overhead<br>
> > > anyway. If cmake parsing (either cmake's or our's) is not fast enough is<br>
> > not<br>
> > > because of this (actually we don't do anything in configure_file).<br>
> > > I don't think it's superfluous anyway.<br>
> ><br>
> > Using a configure-file is good if you have lots of HAVE_XXX in various<br>
> > places/find-modules. But we only have 1 in there, its really, really<br>
> > easy to just have add_definitions( -DHAVE_KOMPARE ) inside the<br>
> > language/CMakeLists.txt and the configure_file doesn't add any value to<br>
> > this - hence its superfluous.<br>
><br>
> Then let's use some of the other files we're already configuring.<br>
<br>
</div>Which other files? The only thing I see is kdevplatformversion.h.cmake and<br>
that one cannot contain such a define because its installed.<br>
<br>
A config.h file is ok, if the define's set in it are used in multiple<br>
places (i.e. add_definitions() would have to be scattered around), but else<br>
its just adding yet another indirection without any added value. With an<br>
add_definition() its instantly clear what happens, with a configure_file I<br>
have to look into the original .cmake file to find out what it configures.<br>
<br>
Andreas<br>
<font color="#888888"><br>
--<br>
You will wish you hadn't.<br>
</font><div><div></div><div class="h5"><br>
_______________________________________________<br>
KDevelop-devel mailing list<br>
<a href="mailto:KDevelop-devel@kdevelop.org">KDevelop-devel@kdevelop.org</a><br>
<a href="https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel" target="_blank">https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel</a><br>
</div></div></blockquote></div><br>Well, as I said, the idea behind that change is that having the code statically inside kdevplatform/language/CMakeLists.txt makes it impossible to reuse that finding on other places, it's not like i go messing with other's code around without a reason, I don't think commiting my local changes on kompare using would be any good either.<br>
<br>We can have a KDEVPLATFORM_DEFINES variable if you prefer. That would be the same as having the configure file. I prefer the configure file though, because it forces it to be explicit by including it.<br><br>Aleix<br>