Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-11-04 Thread Kevin Ottens


 On Oct. 30, 2013, 10:47 a.m., David Faure wrote:
  Looks good to me (apart from the expired pastebin links in the initial 
  description).
  
  I can't Ship it because I'm not a cmake guru/maintainer though.

Same here, looks OK, but I'd like Alex or Stephen to give it the ship it.


- Kevin


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


On Oct. 29, 2013, 10:52 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 29, 2013, 10:52 a.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 File Attachments
 
 
 kparts changes example
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/10/29/a3557579-801b-4ee6-8e3d-5d487428759a__kf5-kparts-headers-test.patch
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-29 Thread Aleix Pol Gonzalez


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 32
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line32
 
  I recommend not putting this in the API of the function, and instead 
  users should use 
  
   install(DIRECTORY) to install the generated files.
 
 Aleix Pol Gonzalez wrote:
 Why?
 
 Stephen Kelly wrote:
 I think it's better API. It's what all existing functions which generate 
 headers do.
 
 Aleix Pol Gonzalez wrote:
 Then it's weird because we'll need to have a separate call for installing 
 all the headers as well, so we will have them all listed twice.
 
 Stephen Kelly wrote:
 Are you sure? You wouldn't just have a single install(DIRECTORY), as I 
 wrote before?

See the KParts patch I attached. We want to install the actual header, not 
the forward header, right?


- Aleix


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


On Oct. 29, 2013, 12:32 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 29, 2013, 12:32 a.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 File Attachments
 
 
 example of how KParts would change
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/10/29/43e2c958-9908-4249-9bb6-9395d91b5643__kf5-kparts-headers-test.patch
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-28 Thread Aleix Pol Gonzalez

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

(Updated Oct. 29, 2013, 12:32 a.m.)


Review request for Build System, KDE Frameworks and Stephen Kelly.


Repository: extra-cmake-modules


Description
---

Create a macro that will generate the forward headers like we used to, in cmake 
configure time.

Here's an example of how we'd port KParts to that macro: 
http://paste.opensuse.org/9880051
After the change, these are the installed headers: 
http://paste.opensuse.org/90980400


Diffs
-

  modules/ECMGenerateHeaders.cmake PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/113406/diff/


Testing
---

Ported KParts and still everything works.


File Attachments


example of how KParts would change
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/10/29/43e2c958-9908-4249-9bb6-9395d91b5643__kf5-kparts-headers-test.patch


Thanks,

Aleix Pol Gonzalez

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-27 Thread David Faure

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


Thanks for working on this!


modules/ECMGenerateHeaders.cmake
http://git.reviewboard.kde.org/r/113406/#comment30804

is used += to



modules/ECMGenerateHeaders.cmake
http://git.reviewboard.kde.org/r/113406/#comment30805

This variable name isn't intuitive, there are headers everywhere (source, 
binary, install dir). Maybe call it SOURCE_DIR or HEADERS_SOURCE_DIR?


- David Faure


On Oct. 26, 2013, 10:56 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 26, 2013, 10:56 a.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-26 Thread Aleix Pol Gonzalez


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 29
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line29
 
  This variable shouldn't be needed at all.
 
 Aleix Pol Gonzalez wrote:
 Variable? Or argument? Why?
 
 Stephen Kelly wrote:
 Argument, yes, sorry.
 
 I think that because the source dir is already in the 
 INTERFACE_INCLUDE_DIRECTORIES of the target, there is no need to have to 
 specify it, and there should be no need to use ${EGH_HEADERS_DIR}/ in the 
 generated include.
 
 I guess you have some reason for doing that, but that just points to 
 non-uniformity which should maybe be fixed, as I wrote in the KIO commit I 
 linked to.
 
 Alexander Neundorf wrote:
 I don't see any target involved here...

So maybe we should link the generated headers to a target?


- Aleix


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


On Oct. 24, 2013, 1:40 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 24, 2013, 1:40 p.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-26 Thread Aleix Pol Gonzalez

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

(Updated Oct. 26, 2013, 9:35 a.m.)


Review request for Build System, KDE Frameworks and Stephen Kelly.


Changes
---

Remove the install functionality, and the INSTALL_DIR argument with it, cmake 
can install the files quite easily anyway.


Repository: extra-cmake-modules


Description
---

Create a macro that will generate the forward headers like we used to, in cmake 
configure time.

Here's an example of how we'd port KParts to that macro: 
http://paste.opensuse.org/9880051
After the change, these are the installed headers: 
http://paste.opensuse.org/90980400


Diffs (updated)
-

  modules/ECMGenerateHeaders.cmake PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/113406/diff/


Testing
---

Ported KParts and still everything works.


Thanks,

Aleix Pol Gonzalez

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-26 Thread Alexander Neundorf

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


The install dir is still in the documentation and in the 
cmake_parse_arguments() call.


- Alexander Neundorf


On Oct. 26, 2013, 9:35 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 26, 2013, 9:35 a.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-26 Thread Aleix Pol Gonzalez

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

(Updated Oct. 26, 2013, 10:56 a.m.)


Review request for Build System, KDE Frameworks and Stephen Kelly.


Changes
---

My bad, I uploaded the wrong patch version.


Repository: extra-cmake-modules


Description
---

Create a macro that will generate the forward headers like we used to, in cmake 
configure time.

Here's an example of how we'd port KParts to that macro: 
http://paste.opensuse.org/9880051
After the change, these are the installed headers: 
http://paste.opensuse.org/90980400


Diffs (updated)
-

  modules/ECMGenerateHeaders.cmake PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/113406/diff/


Testing
---

Ported KParts and still everything works.


Thanks,

Aleix Pol Gonzalez

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-24 Thread Aleix Pol Gonzalez

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

(Updated Oct. 24, 2013, 1:40 p.m.)


Review request for Build System, KDE Frameworks and Stephen Kelly.


Changes
---

Adopt changes proposed by Neundorf.
- Improved documentation.
- Added different arguments to configure how the macro works.


Repository: extra-cmake-modules


Description
---

Create a macro that will generate the forward headers like we used to, in cmake 
configure time.

Here's an example of how we'd port KParts to that macro: 
http://paste.opensuse.org/9880051
After the change, these are the installed headers: 
http://paste.opensuse.org/90980400


Diffs (updated)
-

  modules/ECMGenerateHeaders.cmake PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/113406/diff/


Testing
---

Ported KParts and still everything works.


Thanks,

Aleix Pol Gonzalez

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-24 Thread Stephen Kelly

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



modules/ECMGenerateHeaders.cmake
http://git.reviewboard.kde.org/r/113406/#comment30766

I really think the answers to my questions here need to be found first:

 http://thread.gmane.org/gmane.comp.kde.cvs/1272841

It should be more-simple than it currently is.



modules/ECMGenerateHeaders.cmake
http://git.reviewboard.kde.org/r/113406/#comment30767

This variable shouldn't be needed at all.



modules/ECMGenerateHeaders.cmake
http://git.reviewboard.kde.org/r/113406/#comment30768

I recommend not putting this in the API of the function, and instead users 
should use 

 install(DIRECTORY) to install the generated files.


- Stephen Kelly


On Oct. 24, 2013, 1:40 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 24, 2013, 1:40 p.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-24 Thread Aleix Pol Gonzalez


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 29
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line29
 
  This variable shouldn't be needed at all.

Variable? Or argument? Why?


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 32
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line32
 
  I recommend not putting this in the API of the function, and instead 
  users should use 
  
   install(DIRECTORY) to install the generated files.

Why?


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 11
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line11
 
  I really think the answers to my questions here need to be found first:
  
   http://thread.gmane.org/gmane.comp.kde.cvs/1272841
  
  It should be more-simple than it currently is.

I think that this patch answers most of these questions, leaving up to 
discussion if we expect people to include module/something.h or just 
something.h.

I'd say that if Qt5 transition teaches something is that it's not very flexible 
to namespace the include files, but that's up to the developer anyway...


- Aleix


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


On Oct. 24, 2013, 1:40 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 24, 2013, 1:40 p.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-24 Thread Stephen Kelly


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 11
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line11
 
  I really think the answers to my questions here need to be found first:
  
   http://thread.gmane.org/gmane.comp.kde.cvs/1272841
  
  It should be more-simple than it currently is.
 
 Aleix Pol Gonzalez wrote:
 I think that this patch answers most of these questions, leaving up to 
 discussion if we expect people to include module/something.h or just 
 something.h.
 
 I'd say that if Qt5 transition teaches something is that it's not very 
 flexible to namespace the include files, but that's up to the developer 
 anyway...

I'm not sure it does...

For example, grantlee installs headers like this:

 include/grantlee/engine.h
 include/grantlee_export.h

include/grantlee/engine.h has include grantlee_export.h and even if it is 
grantlee_export.h, the intent is that the user only needs -Iinclude/, and 
does not need -Iinclude/grantlee.

The same is not true of where KF5 installs headers and how they are included. 
There is scope for improvement there, and this patch does not affect that.


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 29
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line29
 
  This variable shouldn't be needed at all.
 
 Aleix Pol Gonzalez wrote:
 Variable? Or argument? Why?

Argument, yes, sorry.

I think that because the source dir is already in the 
INTERFACE_INCLUDE_DIRECTORIES of the target, there is no need to have to 
specify it, and there should be no need to use ${EGH_HEADERS_DIR}/ in the 
generated include.

I guess you have some reason for doing that, but that just points to 
non-uniformity which should maybe be fixed, as I wrote in the KIO commit I 
linked to.


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 32
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line32
 
  I recommend not putting this in the API of the function, and instead 
  users should use 
  
   install(DIRECTORY) to install the generated files.
 
 Aleix Pol Gonzalez wrote:
 Why?

I think it's better API. It's what all existing functions which generate 
headers do.


- Stephen


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


On Oct. 24, 2013, 1:40 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 24, 2013, 1:40 p.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-24 Thread Alexander Neundorf


 On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
  modules/ECMGenerateHeaders.cmake, line 29
  http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line29
 
  This variable shouldn't be needed at all.
 
 Aleix Pol Gonzalez wrote:
 Variable? Or argument? Why?
 
 Stephen Kelly wrote:
 Argument, yes, sorry.
 
 I think that because the source dir is already in the 
 INTERFACE_INCLUDE_DIRECTORIES of the target, there is no need to have to 
 specify it, and there should be no need to use ${EGH_HEADERS_DIR}/ in the 
 generated include.
 
 I guess you have some reason for doing that, but that just points to 
 non-uniformity which should maybe be fixed, as I wrote in the KIO commit I 
 linked to.

I don't see any target involved here...


- Alexander


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


On Oct. 24, 2013, 1:40 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 24, 2013, 1:40 p.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Review Request 113406: Add a macro to automatically generate forward headers

2013-10-23 Thread Aleix Pol Gonzalez

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

Review request for Build System, KDE Frameworks and Stephen Kelly.


Repository: extra-cmake-modules


Description
---

Create a macro that will generate the forward headers like we used to, in cmake 
configure time.

Here's an example of how we'd port KParts to that macro: 
http://paste.opensuse.org/9880051
After the change, these are the installed headers: 
http://paste.opensuse.org/90980400


Diffs
-

  modules/ECMGenerateHeaders.cmake PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/113406/diff/


Testing
---

Ported KParts and still everything works.


Thanks,

Aleix Pol Gonzalez

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem


Re: Review Request 113406: Add a macro to automatically generate forward headers

2013-10-23 Thread Alexander Neundorf

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


IMO the documentation could be improved.
It should mention the assumption about the location of ${actualheader}.
It should also mention the use of ${PROJECT_NAME}.
It needs to mention that it assumes that a variable ${INCLUDE_INSTALL_DIR} is 
set, and that this is used for installing the generated headers.

I often found that a function/macro started like this, with just a list of 
arguments, and later on additional options became necessary (the use of 
PROJECT_NAME and the install dir are candidates here). This is hard to do 
without having keywords in the arguments which separate the different options.
It would be nice if the signature could be changed to
ecm_generate_headers(CLASSES ClassA...ClassN [MODULE_NAME SomeName] 
[INSTALL_DIR /some/dir] ...)
using the cmake_parse_arguments() function.


- Alexander Neundorf


On Oct. 23, 2013, 5:38 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113406/
 ---
 
 (Updated Oct. 23, 2013, 5:38 p.m.)
 
 
 Review request for Build System, KDE Frameworks and Stephen Kelly.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Create a macro that will generate the forward headers like we used to, in 
 cmake configure time.
 
 Here's an example of how we'd port KParts to that macro: 
 http://paste.opensuse.org/9880051
 After the change, these are the installed headers: 
 http://paste.opensuse.org/90980400
 
 
 Diffs
 -
 
   modules/ECMGenerateHeaders.cmake PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/113406/diff/
 
 
 Testing
 ---
 
 Ported KParts and still everything works.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem