From kde-core-devel Mon Oct 07 11:50:34 2013 From: "Stephen Kelly" Date: Mon, 07 Oct 2013 11:50:34 +0000 To: kde-core-devel Subject: Re: Review Request 113139: Try to export include targets for Plasma as well Message-Id: <20131007115034.367.19362 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=138114667031354 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============6240733053171750194==" --===============6240733053171750194== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Oct. 7, 2013, 9:35 a.m., Stephen Kelly wrote: > > src/plasma/CMakeLists.txt, line 173 > > > > > > The if-else shouldn't be needed. INSTALL_INTERFACE should already check if ${INCLUDE_INSTALL_DIR} is absolute. > > Ben Cooksley wrote: > I copied this code from KCoreAddons in kdelibs[frameworks]. Shall I correct it there as well? It appears in several other frameworks too. Actually your snippet is needed until CMake 2.8.12. I'll remove it from them all when kde requires that version. - Stephen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113139/#review41324 ----------------------------------------------------------- On Oct. 7, 2013, 10:48 a.m., Ben Cooksley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113139/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2013, 10:48 a.m.) > > > Review request for kdelibs, Plasma and Stephen Kelly. > > > Repository: plasma-framework > > > Description > ------- > > Add include targets for KF5::plasma, which will hopefully contribute towards fixing the kde-workspace[master] build on build.kde.org. > Unfortunately it isn't entirely successful as it seems camelcase headers are installed by KF5::plasma into include/KDE/Plasma/ but it should be a start... > > > Diffs > ----- > > src/plasma/CMakeLists.txt b21fd7b > > Diff: http://git.reviewboard.kde.org/r/113139/diff/ > > > Testing > ------- > > In place on CI build system. Proper include path now given to compiler. > > > Thanks, > > Ben Cooksley > > --===============6240733053171750194== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113139/

On October 7th, 2013, 9:35 a.m. UTC, Stephen Kelly wrote:
src/plasma/CMakeLists.txt (Diff revision 1)
173
if(IS_ABSOLUTE "${INCLUDE_INSTALL_DIR}")
The if-else shouldn't be needed. INSTALL_INTERFACE should already check if ${INCLUDE_INSTALL_DIR} is absolute.
On October 7th, 2013, 9:36 a.m. UTC, Ben Cooksley wrote:
I copied this code from KCoreAddons in kdelibs[frameworks]. Shall I correct it there as well?

It appears in several other frameworks too. Actually your snippet is needed until CMake 2.8.12. I'll remove it from them all when kde requires that version.

- Stephen

On October 7th, 2013, 10:48 a.m. UTC, Ben Cooksley wrote:

Review request for kdelibs, Plasma and Stephen Kelly.

By Ben Cooksley.

Updated Oct. 7, 2013, 10:48 a.m.

Repository: plasma-framework

Description

Add include targets for KF5::plasma, which will hopefully contribute towards fixing the kde-workspace[master] build on build.kde.org.
Unfortunately it isn't entirely successful as it seems camelcase headers are installed by KF5::plasma into include/KDE/Plasma/ but it should be a start...

Testing

In place on CI build system. Proper include path now given to compiler.

Diffs

src/plasma/CMakeLists.txt (b21fd7b)

--===============6240733053171750194==--