KDE/kdevplatform

[Aleix Pol]

On Tue, Aug 4, 2009 at 1:39 PM, Andreas Pakulat <apaku at gmx.de> wrote:

> On 04.08.09 12:57:42, Aleix Pol wrote:
> > On Tue, Aug 4, 2009 at 9:18 AM, Andreas Pakulat <apaku at gmx.de> wrote:
> > > > > > +configure_file(
> > > ${CMAKE_CURRENT_SOURCE_DIR}/config-kdevplatform.h.cmake
> > > > > > +
>  ${CMAKE_CURRENT_BINARY_DIR}/config-kdevplatform.h )
> > > > > > +
> > > > >
> > > > > Even more code to execute.
> > > >
> > > > I wonder what you mean. Every single cmake project use configure file
> for
> > > > conditional compilation inside files. I don't think is that much
> overhead
> > > > anyway. If cmake parsing (either cmake's or our's) is not fast enough
> is
> > > not
> > > > because of this (actually we don't do anything in configure_file).
> > > > I don't think it's superfluous anyway.
> > >
> > > Using a configure-file is good if you have lots of HAVE_XXX in various
> > > places/find-modules. But we only have 1 in there, its really, really
> > > easy to just have add_definitions( -DHAVE_KOMPARE ) inside the
> > > language/CMakeLists.txt and the configure_file doesn't add any value to
> > > this - hence its superfluous.
> >
> > Then let's use some of the other files we're already configuring.
>
> Which other files? The only thing I see is kdevplatformversion.h.cmake and
> that one cannot contain such a define because its installed.
>
> A config.h file is ok, if the define's set in it are used in multiple
> places (i.e. add_definitions() would have to be scattered around), but else
> its just adding yet another indirection without any added value. With an
> add_definition() its instantly clear what happens, with a configure_file I
> have to look into the original .cmake file to find out what it configures.
>
> Andreas
>
> --
> You will wish you hadn't.
>
> _______________________________________________
> KDevelop-devel mailing list
> KDevelop-devel at kdevelop.org
> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>

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.

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.

Aleix
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20090804/65fe1f50/attachment.html>