Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-07-08 Thread Michael Palimaka 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61923
---


Build is failing now when BUILD_EXAMPLES is turned on.

- Michael Palimaka


On July 7, 2014, 7:28 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated July 7, 2014, 7:28 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-07-07 Thread Hrvoje Senjan 

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

(Updated July 7, 2014, 7:28 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
Christophe Giboudeaux.


Repository: polkit-qt-1


Description
---

added exported targets, simplify main CMakeLists...


Diffs
-

  CMakeLists.txt 021bf88 
  PolkitQt-1Config.cmake.in 8b722a6 
  agent/CMakeLists.txt f1ba438 
  core/CMakeLists.txt e9b3ebb 
  gui/CMakeLists.txt 10b06ae 
  polkit-qt-1.pc.cmake 2f33204 
  polkit-qt-agent-1.pc.cmake 6ccc6dd 
  polkit-qt-core-1.pc.cmake a9e0750 
  polkit-qt-gui-1.pc.cmake 6b9c2cf 
  polkit-qt5-1.pc.cmake PRE-CREATION 
  polkit-qt5-agent-1.pc.cmake PRE-CREATION 
  polkit-qt5-core-1.pc.cmake PRE-CREATION 
  polkit-qt5-gui-1.pc.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119043/diff/


Testing
---

builds, cmake files visually inspected, seem fine


Thanks,

Hrvoje Senjan

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-07-06 Thread Aleix Pol Gonzalez 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61766
---

Ship it!


Looks good to me.

- Aleix Pol Gonzalez


On July 1, 2014, 6:23 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated July 1, 2014, 6:23 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-07-01 Thread Hrvoje Senjan 

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

(Updated July 1, 2014, 8:23 p.m.)


Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
Christophe Giboudeaux.


Changes
---

improved exported names


Repository: polkit-qt-1


Description
---

added exported targets, simplify main CMakeLists...


Diffs (updated)
-

  CMakeLists.txt 021bf88 
  PolkitQt-1Config.cmake.in 8b722a6 
  agent/CMakeLists.txt f1ba438 
  core/CMakeLists.txt e9b3ebb 
  gui/CMakeLists.txt 10b06ae 
  polkit-qt-1.pc.cmake 2f33204 
  polkit-qt-agent-1.pc.cmake 6ccc6dd 
  polkit-qt-core-1.pc.cmake a9e0750 
  polkit-qt-gui-1.pc.cmake 6b9c2cf 
  polkit-qt5-1.pc.cmake PRE-CREATION 
  polkit-qt5-agent-1.pc.cmake PRE-CREATION 
  polkit-qt5-core-1.pc.cmake PRE-CREATION 
  polkit-qt5-gui-1.pc.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119043/diff/


Testing
---

builds, cmake files visually inspected, seem fine


Thanks,

Hrvoje Senjan

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-07-01 Thread Hrvoje Senjan 


> On July 1, 2014, 1:43 a.m., Aleix Pol Gonzalez wrote:
> > core/CMakeLists.txt, line 24
> > 
> >
> > Maybe we want to set an EXPORT_NAME? This way the user won't need a 
> > variable to link to the library.
> > 
> > It's better linking against libraries rather than variables because you 
> > don't risk adding a typo and resolving to an empty string.
> 
> Hrvoje Senjan wrote:
> we have export just a few lines below, so you can link to 
> PolkitQt5-1::polkit-qt5-core-1... or i completely missed your point?
> other issues, to me looks could be droped? will re-add quotes to 
> POLKITQT-1_VERSION_STRING. for the rest: as this is still both Qt4 and Qt5 
> library in the same branch, i would really like to avoid not needed changes. 
> That can be done once Qt4 support is gone/moved to branch.
> 
> Aleix Pol Gonzalez wrote:
> Well, I don't think any healthy person would want to type anything like 
> "PolkitQt5-1::polkit-qt5-core-1", at least PolkitQt5-1::Core.
> 
> That won't mean changes on currently running applications because these 
> should be using the variable anyway.

right. will update soon™


- Hrvoje


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61334
---


On June 30, 2014, 11:53 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 11:53 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-07-01 Thread Aleix Pol Gonzalez 


> On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote:
> > core/CMakeLists.txt, line 24
> > 
> >
> > Maybe we want to set an EXPORT_NAME? This way the user won't need a 
> > variable to link to the library.
> > 
> > It's better linking against libraries rather than variables because you 
> > don't risk adding a typo and resolving to an empty string.
> 
> Hrvoje Senjan wrote:
> we have export just a few lines below, so you can link to 
> PolkitQt5-1::polkit-qt5-core-1... or i completely missed your point?
> other issues, to me looks could be droped? will re-add quotes to 
> POLKITQT-1_VERSION_STRING. for the rest: as this is still both Qt4 and Qt5 
> library in the same branch, i would really like to avoid not needed changes. 
> That can be done once Qt4 support is gone/moved to branch.

Well, I don't think any healthy person would want to type anything like 
"PolkitQt5-1::polkit-qt5-core-1", at least PolkitQt5-1::Core.

That won't mean changes on currently running applications because these should 
be using the variable anyway.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61334
---


On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 9:53 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-07-01 Thread Hrvoje Senjan 


> On July 1, 2014, 1:43 a.m., Aleix Pol Gonzalez wrote:
> > core/CMakeLists.txt, line 24
> > 
> >
> > Maybe we want to set an EXPORT_NAME? This way the user won't need a 
> > variable to link to the library.
> > 
> > It's better linking against libraries rather than variables because you 
> > don't risk adding a typo and resolving to an empty string.

we have export just a few lines below, so you can link to 
PolkitQt5-1::polkit-qt5-core-1... or i completely missed your point?
other issues, to me looks could be droped? will re-add quotes to 
POLKITQT-1_VERSION_STRING. for the rest: as this is still both Qt4 and Qt5 
library in the same branch, i would really like to avoid not needed changes. 
That can be done once Qt4 support is gone/moved to branch.


- Hrvoje


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61334
---


On June 30, 2014, 11:53 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 11:53 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Aleix Pol Gonzalez 


> On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote:
> > PolkitQt-1Config.cmake.in, line 24
> > 
> >
> > Passing the include dir is not needed anymore, given that those will be 
> > pulled by the targets.
> > 
> > I suggest leaving them empty (or even not defining them). It won't stop 
> > applications from building, it will just resolve to nothing.
> 
> Christophe Giboudeaux wrote:
> ...but would not be SC. Anything using POLKITQT-1_INCLUDE_DIR would fail 
> to build.
>

Only if it doesn't link against polkitqt-1.


> On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote:
> > CMakeLists.txt, line 6
> > 
> >
> > Isn't it acceptable to depend on ECM here?
> 
> Christophe Giboudeaux wrote:
> not a wise choice. the master branch allows building both the Qt4 & Qt5 
> variant (and depends on CMake 2.8.11)

I disagree, but accept your premise.


> On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote:
> > core/CMakeLists.txt, line 13
> > 
> >
> > Just use PUBLIC and PRIVATE, like the rest of frameworks
> 
> Christophe Giboudeaux wrote:
> PUBLIC and PRIVATE do not exist in cmake 2.8.11.

Ok, withdraw this complaint.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61334
---


On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 9:53 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Christophe Giboudeaux 


> On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote:
> > CMakeLists.txt, line 6
> > 
> >
> > Isn't it acceptable to depend on ECM here?

not a wise choice. the master branch allows building both the Qt4 & Qt5 variant 
(and depends on CMake 2.8.11)


> On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote:
> > core/CMakeLists.txt, line 13
> > 
> >
> > Just use PUBLIC and PRIVATE, like the rest of frameworks

PUBLIC and PRIVATE do not exist in cmake 2.8.11.


> On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote:
> > PolkitQt-1Config.cmake.in, line 24
> > 
> >
> > Passing the include dir is not needed anymore, given that those will be 
> > pulled by the targets.
> > 
> > I suggest leaving them empty (or even not defining them). It won't stop 
> > applications from building, it will just resolve to nothing.

...but would not be SC. Anything using POLKITQT-1_INCLUDE_DIR would fail to 
build.


- Christophe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61334
---


On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 9:53 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Aleix Pol Gonzalez 


> On June 30, 2014, 8:37 p.m., Christophe Giboudeaux wrote:
> > agent/CMakeLists.txt, line 10
> > 
> >
> > LINK_PUBLIC


> On June 30, 2014, 8:37 p.m., Christophe Giboudeaux wrote:
> > core/CMakeLists.txt, line 13
> > 
> >
> > PUBLIC was added in CMake 2.8.12, use LINK_PUBLIC

So? all frameworks is depending on cmake 2.8.12, and LINK_PUBLIC is considered 
deprecated there.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61326
---


On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 9:53 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Aleix Pol Gonzalez 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61334
---



CMakeLists.txt


Isn't it acceptable to depend on ECM here?



PolkitQt-1Config.cmake.in


why's that change?




PolkitQt-1Config.cmake.in


Passing the include dir is not needed anymore, given that those will be 
pulled by the targets.

I suggest leaving them empty (or even not defining them). It won't stop 
applications from building, it will just resolve to nothing.



PolkitQt-1Config.cmake.in


why don't you just do something like:
set(POLKITQT-1_CORE_LIBRARY ${POLKITQT-1_CORE_PCNAME})

This should work on all platforms.



core/CMakeLists.txt


Just use PUBLIC and PRIVATE, like the rest of frameworks



core/CMakeLists.txt


Maybe we want to set an EXPORT_NAME? This way the user won't need a 
variable to link to the library.

It's better linking against libraries rather than variables because you 
don't risk adding a typo and resolving to an empty string.


- Aleix Pol Gonzalez


On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 9:53 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
>   polkit-qt5-1.pc.cmake PRE-CREATION 
>   polkit-qt5-agent-1.pc.cmake PRE-CREATION 
>   polkit-qt5-core-1.pc.cmake PRE-CREATION 
>   polkit-qt5-gui-1.pc.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Hrvoje Senjan 

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

(Updated June 30, 2014, 11:53 p.m.)


Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
Christophe Giboudeaux.


Changes
---

added qt5 pc files


Repository: polkit-qt-1


Description
---

added exported targets, simplify main CMakeLists...


Diffs (updated)
-

  CMakeLists.txt 021bf88 
  PolkitQt-1Config.cmake.in 8b722a6 
  agent/CMakeLists.txt f1ba438 
  core/CMakeLists.txt e9b3ebb 
  gui/CMakeLists.txt 10b06ae 
  polkit-qt-1.pc.cmake 2f33204 
  polkit-qt-agent-1.pc.cmake 6ccc6dd 
  polkit-qt-core-1.pc.cmake a9e0750 
  polkit-qt-gui-1.pc.cmake 6b9c2cf 
  polkit-qt5-1.pc.cmake PRE-CREATION 
  polkit-qt5-agent-1.pc.cmake PRE-CREATION 
  polkit-qt5-core-1.pc.cmake PRE-CREATION 
  polkit-qt5-gui-1.pc.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119043/diff/


Testing
---

builds, cmake files visually inspected, seem fine


Thanks,

Hrvoje Senjan

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Hrvoje Senjan 

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

(Updated June 30, 2014, 10:51 p.m.)


Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
Christophe Giboudeaux.


Changes
---

resolved issues


Repository: polkit-qt-1


Description
---

added exported targets, simplify main CMakeLists...


Diffs (updated)
-

  CMakeLists.txt 021bf88 
  PolkitQt-1Config.cmake.in 8b722a6 
  agent/CMakeLists.txt f1ba438 
  core/CMakeLists.txt e9b3ebb 
  gui/CMakeLists.txt 10b06ae 
  polkit-qt-1.pc.cmake 2f33204 
  polkit-qt-agent-1.pc.cmake 6ccc6dd 
  polkit-qt-core-1.pc.cmake a9e0750 
  polkit-qt-gui-1.pc.cmake 6b9c2cf 

Diff: https://git.reviewboard.kde.org/r/119043/diff/


Testing
---

builds, cmake files visually inspected, seem fine


Thanks,

Hrvoje Senjan

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Christophe Giboudeaux 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61326
---



agent/CMakeLists.txt


LINK_PUBLIC



agent/CMakeLists.txt


LINK_PRIVATE



core/CMakeLists.txt


PUBLIC was added in CMake 2.8.12, use LINK_PUBLIC



core/CMakeLists.txt


and LINK_PRIVATE



core/CMakeLists.txt


Not needed anymore



gui/CMakeLists.txt


LINK_PUBLIC



gui/CMakeLists.txt


LINK_PRIVATE


- Christophe Giboudeaux


On June 30, 2014, 8:28 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 8:28 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
>   polkit-qt-1.pc.cmake 2f33204 
>   polkit-qt-agent-1.pc.cmake 6ccc6dd 
>   polkit-qt-core-1.pc.cmake a9e0750 
>   polkit-qt-gui-1.pc.cmake 6b9c2cf 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Hrvoje Senjan 

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

(Updated June 30, 2014, 10:28 p.m.)


Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
Christophe Giboudeaux.


Changes
---

adjusted pc files paths.
added ${GOBJECT_LIBRARIES} and ${GIO_LIBRARIES} to agent's link libraries: the 
build fails otherwise with -Wl, --no-undefined
requires would be great to fix, i agree, just need to think of the easiest way 
to cover both Qt4 and Qt5 cases. maybe separate pc files...


Repository: polkit-qt-1


Description
---

added exported targets, simplify main CMakeLists...


Diffs (updated)
-

  CMakeLists.txt 021bf88 
  PolkitQt-1Config.cmake.in 8b722a6 
  agent/CMakeLists.txt f1ba438 
  core/CMakeLists.txt e9b3ebb 
  gui/CMakeLists.txt 10b06ae 
  polkit-qt-1.pc.cmake 2f33204 
  polkit-qt-agent-1.pc.cmake 6ccc6dd 
  polkit-qt-core-1.pc.cmake a9e0750 
  polkit-qt-gui-1.pc.cmake 6b9c2cf 

Diff: https://git.reviewboard.kde.org/r/119043/diff/


Testing
---

builds, cmake files visually inspected, seem fine


Thanks,

Hrvoje Senjan

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

Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements

2014-06-30 Thread Christophe Giboudeaux 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61318
---


The pkgconfig files need to be changed as well.
While changing the paths, fixing the 'Requires:' lines would be a good idea.

The rest looks correct.



- Christophe Giboudeaux


On June 30, 2014, 6:19 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119043/
> ---
> 
> (Updated June 30, 2014, 6:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and 
> Christophe Giboudeaux.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> added exported targets, simplify main CMakeLists...
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 021bf88 
>   PolkitQt-1Config.cmake.in 8b722a6 
>   agent/CMakeLists.txt f1ba438 
>   core/CMakeLists.txt e9b3ebb 
>   gui/CMakeLists.txt 10b06ae 
> 
> Diff: https://git.reviewboard.kde.org/r/119043/diff/
> 
> 
> Testing
> ---
> 
> builds, cmake files visually inspected, seem fine
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

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