Re: Review Request 123477: Add public dependency on NetworkManager

2015-04-23 Thread Aleix Pol Gonzalez


On April 23, 2015, 3:35 p.m., Jan Grulich wrote:
  And yes, this is required as long as NetworkManager headers are included by 
  NetworkManagerQt headers.
 
 Jan Grulich wrote:
 I just found out that installing FindNetworkManager.cmake into CMake 
 modules is not enough, because it still cannot find it. I guess this would 
 have to be done according to other FindFoo.cmake modules from 
 extra-cmake-modules, right?

That would certainly help, but then it's not very nice putting a cmake file 
into another project's directory.
I would suggest to either:

* Contribute FindNetworkManager.cmake to extra-cmake-modules
* Install it within the NetworkManagerQt cmake directory, together with 
NetworkManagerQtConfig.cmake.


- Aleix


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


On April 23, 2015, 3:01 p.m., Jan Grulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123477/
 ---
 
 (Updated April 23, 2015, 3:01 p.m.)
 
 
 Review request for KDE Frameworks, David Faure, Daniel Vrátil, and Lamarque 
 Souza.
 
 
 Repository: networkmanager-qt
 
 
 Description
 ---
 
 I'm not 100% sure how this should work and I couldn't find another framework 
 doing the same, but Daniel Vrátil pointed me out that NetworkManagerQt should 
 mention in KF5NetworkManagerQtConfig.cmake file that it requires 
 NetworkManager as dependency. Given this, we also need to install 
 FindNetworkManager.cmake into CMake modules so NetworkManager can be found by 
 find_dependency() macro, or FindNetworkManager.cmake can go into 
 extra-cmake-modules.
 
 If this is how it should be done, then similar patch would be needed for 
 ModemManagerQt.
 
 
 Diffs
 -
 
   CMakeLists.txt c9e3274 
   KF5NetworkManagerQtConfig.cmake.in cdabe8e 
 
 Diff: https://git.reviewboard.kde.org/r/123477/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jan Grulich
 


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


Re: Review Request 123477: Add public dependency on NetworkManager

2015-04-23 Thread Lamarque Souza


On April 23, 2015, 1:35 p.m., Jan Grulich wrote:
  And yes, this is required as long as NetworkManager headers are included by 
  NetworkManagerQt headers.
 
 Jan Grulich wrote:
 I just found out that installing FindNetworkManager.cmake into CMake 
 modules is not enough, because it still cannot find it. I guess this would 
 have to be done according to other FindFoo.cmake modules from 
 extra-cmake-modules, right?
 
 Aleix Pol Gonzalez wrote:
 That would certainly help, but then it's not very nice putting a cmake 
 file into another project's directory.
 I would suggest to either:
 
 * Contribute FindNetworkManager.cmake to extra-cmake-modules
 * Install it within the NetworkManagerQt cmake directory, together with 
 NetworkManagerQtConfig.cmake.
 
 Jan Grulich wrote:
 I actually didn't mean that I should install it to ../ECM/modules, but 
 whether using find_package_handle_standard_args() and other macros, which are 
 used in all FindFoo.cmake modules from e-c-m, are required to make this work.
 
 Aleix Pol Gonzalez wrote:
 Not required per se, but recommended. There's many things to be defined 
 on a proper cmake finder.
 OTOH, this finder seems to be working already.

I would go for moving FindNetworkManager.cmake to e-c-m.


- Lamarque


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


On April 23, 2015, 1:01 p.m., Jan Grulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123477/
 ---
 
 (Updated April 23, 2015, 1:01 p.m.)
 
 
 Review request for KDE Frameworks, David Faure, Daniel Vrátil, and Lamarque 
 Souza.
 
 
 Repository: networkmanager-qt
 
 
 Description
 ---
 
 I'm not 100% sure how this should work and I couldn't find another framework 
 doing the same, but Daniel Vrátil pointed me out that NetworkManagerQt should 
 mention in KF5NetworkManagerQtConfig.cmake file that it requires 
 NetworkManager as dependency. Given this, we also need to install 
 FindNetworkManager.cmake into CMake modules so NetworkManager can be found by 
 find_dependency() macro, or FindNetworkManager.cmake can go into 
 extra-cmake-modules.
 
 If this is how it should be done, then similar patch would be needed for 
 ModemManagerQt.
 
 
 Diffs
 -
 
   CMakeLists.txt c9e3274 
   KF5NetworkManagerQtConfig.cmake.in cdabe8e 
 
 Diff: https://git.reviewboard.kde.org/r/123477/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jan Grulich
 


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


Re: Review Request 123477: Add public dependency on NetworkManager

2015-04-23 Thread Aleix Pol Gonzalez


On April 23, 2015, 3:35 p.m., Jan Grulich wrote:
  And yes, this is required as long as NetworkManager headers are included by 
  NetworkManagerQt headers.
 
 Jan Grulich wrote:
 I just found out that installing FindNetworkManager.cmake into CMake 
 modules is not enough, because it still cannot find it. I guess this would 
 have to be done according to other FindFoo.cmake modules from 
 extra-cmake-modules, right?
 
 Aleix Pol Gonzalez wrote:
 That would certainly help, but then it's not very nice putting a cmake 
 file into another project's directory.
 I would suggest to either:
 
 * Contribute FindNetworkManager.cmake to extra-cmake-modules
 * Install it within the NetworkManagerQt cmake directory, together with 
 NetworkManagerQtConfig.cmake.
 
 Jan Grulich wrote:
 I actually didn't mean that I should install it to ../ECM/modules, but 
 whether using find_package_handle_standard_args() and other macros, which are 
 used in all FindFoo.cmake modules from e-c-m, are required to make this work.

Not required per se, but recommended. There's many things to be defined on a 
proper cmake finder.
OTOH, this finder seems to be working already.


- Aleix


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


On April 23, 2015, 3:01 p.m., Jan Grulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123477/
 ---
 
 (Updated April 23, 2015, 3:01 p.m.)
 
 
 Review request for KDE Frameworks, David Faure, Daniel Vrátil, and Lamarque 
 Souza.
 
 
 Repository: networkmanager-qt
 
 
 Description
 ---
 
 I'm not 100% sure how this should work and I couldn't find another framework 
 doing the same, but Daniel Vrátil pointed me out that NetworkManagerQt should 
 mention in KF5NetworkManagerQtConfig.cmake file that it requires 
 NetworkManager as dependency. Given this, we also need to install 
 FindNetworkManager.cmake into CMake modules so NetworkManager can be found by 
 find_dependency() macro, or FindNetworkManager.cmake can go into 
 extra-cmake-modules.
 
 If this is how it should be done, then similar patch would be needed for 
 ModemManagerQt.
 
 
 Diffs
 -
 
   CMakeLists.txt c9e3274 
   KF5NetworkManagerQtConfig.cmake.in cdabe8e 
 
 Diff: https://git.reviewboard.kde.org/r/123477/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jan Grulich
 


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


Re: Review Request 123477: Add public dependency on NetworkManager

2015-04-23 Thread Jan Grulich


On Dub. 23, 2015, 1:35 odp., Jan Grulich wrote:
  And yes, this is required as long as NetworkManager headers are included by 
  NetworkManagerQt headers.
 
 Jan Grulich wrote:
 I just found out that installing FindNetworkManager.cmake into CMake 
 modules is not enough, because it still cannot find it. I guess this would 
 have to be done according to other FindFoo.cmake modules from 
 extra-cmake-modules, right?
 
 Aleix Pol Gonzalez wrote:
 That would certainly help, but then it's not very nice putting a cmake 
 file into another project's directory.
 I would suggest to either:
 
 * Contribute FindNetworkManager.cmake to extra-cmake-modules
 * Install it within the NetworkManagerQt cmake directory, together with 
 NetworkManagerQtConfig.cmake.

I actually didn't mean that I should install it to ../ECM/modules, but whether 
using find_package_handle_standard_args() and other macros, which are used in 
all FindFoo.cmake modules from e-c-m, are required to make this work.


- Jan


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


On Dub. 23, 2015, 1:01 odp., Jan Grulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123477/
 ---
 
 (Updated Dub. 23, 2015, 1:01 odp.)
 
 
 Review request for KDE Frameworks, David Faure, Daniel Vrátil, and Lamarque 
 Souza.
 
 
 Repository: networkmanager-qt
 
 
 Description
 ---
 
 I'm not 100% sure how this should work and I couldn't find another framework 
 doing the same, but Daniel Vrátil pointed me out that NetworkManagerQt should 
 mention in KF5NetworkManagerQtConfig.cmake file that it requires 
 NetworkManager as dependency. Given this, we also need to install 
 FindNetworkManager.cmake into CMake modules so NetworkManager can be found by 
 find_dependency() macro, or FindNetworkManager.cmake can go into 
 extra-cmake-modules.
 
 If this is how it should be done, then similar patch would be needed for 
 ModemManagerQt.
 
 
 Diffs
 -
 
   CMakeLists.txt c9e3274 
   KF5NetworkManagerQtConfig.cmake.in cdabe8e 
 
 Diff: https://git.reviewboard.kde.org/r/123477/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jan Grulich
 


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


Re: Review Request 123477: Add public dependency on NetworkManager

2015-04-23 Thread Jan Grulich


 On Dub. 23, 2015, 1:35 odp., Aleix Pol Gonzalez wrote:
  KF5NetworkManagerQtConfig.cmake.in, line 4
  https://git.reviewboard.kde.org/r/123477/diff/1/?file=362685#file362685line4
 
  nitpick. weird 1 space indentation.

Already spotted.


On Dub. 23, 2015, 1:35 odp., Jan Grulich wrote:
  And yes, this is required as long as NetworkManager headers are included by 
  NetworkManagerQt headers.

I just found out that installing FindNetworkManager.cmake into CMake modules is 
not enough, because it still cannot find it. I guess this would have to be done 
according to other FindFoo.cmake modules from extra-cmake-modules, right?


- Jan


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


On Dub. 23, 2015, 1:01 odp., Jan Grulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123477/
 ---
 
 (Updated Dub. 23, 2015, 1:01 odp.)
 
 
 Review request for KDE Frameworks, David Faure, Daniel Vrátil, and Lamarque 
 Souza.
 
 
 Repository: networkmanager-qt
 
 
 Description
 ---
 
 I'm not 100% sure how this should work and I couldn't find another framework 
 doing the same, but Daniel Vrátil pointed me out that NetworkManagerQt should 
 mention in KF5NetworkManagerQtConfig.cmake file that it requires 
 NetworkManager as dependency. Given this, we also need to install 
 FindNetworkManager.cmake into CMake modules so NetworkManager can be found by 
 find_dependency() macro, or FindNetworkManager.cmake can go into 
 extra-cmake-modules.
 
 If this is how it should be done, then similar patch would be needed for 
 ModemManagerQt.
 
 
 Diffs
 -
 
   CMakeLists.txt c9e3274 
   KF5NetworkManagerQtConfig.cmake.in cdabe8e 
 
 Diff: https://git.reviewboard.kde.org/r/123477/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jan Grulich
 


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


Review Request 123477: Add public dependency on NetworkManager

2015-04-23 Thread Jan Grulich

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

Review request for KDE Frameworks, David Faure, Daniel Vrátil, and Lamarque 
Souza.


Repository: networkmanager-qt


Description
---

I'm not 100% sure how this should work and I couldn't find another framework 
doing the same, but Daniel Vrátil pointed me out that NetworkManagerQt should 
mention in KF5NetworkManagerQtConfig.cmake file that it requires NetworkManager 
as dependency. Given this, we also need to install FindNetworkManager.cmake 
into CMake modules so NetworkManager can be found by find_dependency() macro, 
or FindNetworkManager.cmake can go into extra-cmake-modules.

If this is how it should be done, then similar patch would be needed for 
ModemManagerQt.


Diffs
-

  CMakeLists.txt c9e3274 
  KF5NetworkManagerQtConfig.cmake.in cdabe8e 

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


Testing
---


Thanks,

Jan Grulich

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