Re: Review Request 116090: Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

2014-03-15 Thread Alexander Richardson

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

(Updated March 15, 2014, 4:31 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Repository: kde4support


Description
---

Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

This means CMake no longer warns about Policy CMP0026 is not set when
building projects that need kde4support


Diffs
-

  cmake/modules/ECMQt4To5Porting.cmake 4204fa541790aa38c74b9d6f0b2111af2157b2bc 
  cmake/modules/KDE4Macros.cmake 192094b1c5c6877cd9dfa18dc27338c491d753f3 
  src/CMakeLists.txt 6bda0996c118ce212860e02d0505fd3bdc026833 
  src/KDEUIMacros.cmake 1570df340a672a597a0b7480885c340faca0069d 

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


Testing
---

kde4support compiles, kde-workspace compiles


Thanks,

Alexander Richardson

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


Re: Review Request 116090: Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

2014-03-06 Thread Aurélien Gâteau


 On Feb. 27, 2014, 10:54 a.m., Alex Merry wrote:
  The problem with doing this in support code is that it is not strictly 
  source compatible.  An example this would break is if you want to embed the 
  value of QT_QMAKE_EXECUTABLE into a C++ executable using something like
  add_definitions(-DQMAKE=\${QT_QMAKE_EXECUTABLE}\)
  Any use of QMAKE in the program would then expand to 
  $TARGET_FILE:Qt5::qmake, which is obviously not what was wanted.
 
 Alexander Richardson wrote:
 Ah, didn't know that, should I drop this request?
 
 Alex Merry wrote:
 Yeah, I don't think it's worth risking breakage for kde4support.
 
 Aleix Pol Gonzalez wrote:
 Setting the policy instead would probably help though.
 This warning is very verbose and not very useful to the developer either.
 
 Alex Merry wrote:
 Yeah, that seems reasonable.  The best approach is to use 
 cmake_policy(PUSH) and cmake_policy(POP) in the relevant files, so it doesn't 
 affect any of the developer's own code (see 
 http://www.cmake.org/cmake/help/v2.8.12/cmake.html#command:cmake_policy ).
 
 Aurélien Gâteau wrote:
 I just did that here: https://git.reviewboard.kde.org/r/116628/

Actually my code was not as complete as this one, and not as good.

The parts which affect internal variables (such as those in KDE4Macros.cmake) 
can go in. The parts which affect public variables should be changed to instead 
wrap the location getters in cmake policy changes as Alex suggested (ie: 
cmake_policy(PUSH) ; cmake_policy(SET CMP0026 OLD) ; ...get some locations... ; 
cmake_policy(POP))


- Aurélien


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


On Feb. 26, 2014, 6:03 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116090/
 ---
 
 (Updated Feb. 26, 2014, 6:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Use $TARGET_FILE:... instead of get_target_property(... LOCATION)
 
 This means CMake no longer warns about Policy CMP0026 is not set when
 building projects that need kde4support
 
 
 Diffs
 -
 
   cmake/modules/ECMQt4To5Porting.cmake 
 4204fa541790aa38c74b9d6f0b2111af2157b2bc 
   cmake/modules/KDE4Macros.cmake 192094b1c5c6877cd9dfa18dc27338c491d753f3 
   src/CMakeLists.txt 6bda0996c118ce212860e02d0505fd3bdc026833 
   src/KDEUIMacros.cmake 1570df340a672a597a0b7480885c340faca0069d 
 
 Diff: https://git.reviewboard.kde.org/r/116090/diff/
 
 
 Testing
 ---
 
 kde4support compiles, kde-workspace compiles
 
 
 Thanks,
 
 Alexander Richardson
 


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


Re: Review Request 116090: Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

2014-03-05 Thread Alex Merry


 On Feb. 27, 2014, 9:54 a.m., Alex Merry wrote:
  The problem with doing this in support code is that it is not strictly 
  source compatible.  An example this would break is if you want to embed the 
  value of QT_QMAKE_EXECUTABLE into a C++ executable using something like
  add_definitions(-DQMAKE=\${QT_QMAKE_EXECUTABLE}\)
  Any use of QMAKE in the program would then expand to 
  $TARGET_FILE:Qt5::qmake, which is obviously not what was wanted.
 
 Alexander Richardson wrote:
 Ah, didn't know that, should I drop this request?

Yeah, I don't think it's worth risking breakage for kde4support.


- Alex


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


On Feb. 26, 2014, 5:03 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116090/
 ---
 
 (Updated Feb. 26, 2014, 5:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Use $TARGET_FILE:... instead of get_target_property(... LOCATION)
 
 This means CMake no longer warns about Policy CMP0026 is not set when
 building projects that need kde4support
 
 
 Diffs
 -
 
   cmake/modules/ECMQt4To5Porting.cmake 
 4204fa541790aa38c74b9d6f0b2111af2157b2bc 
   cmake/modules/KDE4Macros.cmake 192094b1c5c6877cd9dfa18dc27338c491d753f3 
   src/CMakeLists.txt 6bda0996c118ce212860e02d0505fd3bdc026833 
   src/KDEUIMacros.cmake 1570df340a672a597a0b7480885c340faca0069d 
 
 Diff: https://git.reviewboard.kde.org/r/116090/diff/
 
 
 Testing
 ---
 
 kde4support compiles, kde-workspace compiles
 
 
 Thanks,
 
 Alexander Richardson
 


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


Re: Review Request 116090: Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

2014-03-05 Thread Alex Merry


 On Feb. 27, 2014, 9:54 a.m., Alex Merry wrote:
  The problem with doing this in support code is that it is not strictly 
  source compatible.  An example this would break is if you want to embed the 
  value of QT_QMAKE_EXECUTABLE into a C++ executable using something like
  add_definitions(-DQMAKE=\${QT_QMAKE_EXECUTABLE}\)
  Any use of QMAKE in the program would then expand to 
  $TARGET_FILE:Qt5::qmake, which is obviously not what was wanted.
 
 Alexander Richardson wrote:
 Ah, didn't know that, should I drop this request?
 
 Alex Merry wrote:
 Yeah, I don't think it's worth risking breakage for kde4support.
 
 Aleix Pol Gonzalez wrote:
 Setting the policy instead would probably help though.
 This warning is very verbose and not very useful to the developer either.

Yeah, that seems reasonable.  The best approach is to use cmake_policy(PUSH) 
and cmake_policy(POP) in the relevant files, so it doesn't affect any of the 
developer's own code (see 
http://www.cmake.org/cmake/help/v2.8.12/cmake.html#command:cmake_policy ).


- Alex


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


On Feb. 26, 2014, 5:03 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116090/
 ---
 
 (Updated Feb. 26, 2014, 5:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Use $TARGET_FILE:... instead of get_target_property(... LOCATION)
 
 This means CMake no longer warns about Policy CMP0026 is not set when
 building projects that need kde4support
 
 
 Diffs
 -
 
   cmake/modules/ECMQt4To5Porting.cmake 
 4204fa541790aa38c74b9d6f0b2111af2157b2bc 
   cmake/modules/KDE4Macros.cmake 192094b1c5c6877cd9dfa18dc27338c491d753f3 
   src/CMakeLists.txt 6bda0996c118ce212860e02d0505fd3bdc026833 
   src/KDEUIMacros.cmake 1570df340a672a597a0b7480885c340faca0069d 
 
 Diff: https://git.reviewboard.kde.org/r/116090/diff/
 
 
 Testing
 ---
 
 kde4support compiles, kde-workspace compiles
 
 
 Thanks,
 
 Alexander Richardson
 


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


Re: Review Request 116090: Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

2014-03-04 Thread Alexander Richardson


 On Feb. 27, 2014, 10:54 a.m., Alex Merry wrote:
  The problem with doing this in support code is that it is not strictly 
  source compatible.  An example this would break is if you want to embed the 
  value of QT_QMAKE_EXECUTABLE into a C++ executable using something like
  add_definitions(-DQMAKE=\${QT_QMAKE_EXECUTABLE}\)
  Any use of QMAKE in the program would then expand to 
  $TARGET_FILE:Qt5::qmake, which is obviously not what was wanted.

Ah, didn't know that, should I drop this request?


- Alexander


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


On Feb. 26, 2014, 6:03 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116090/
 ---
 
 (Updated Feb. 26, 2014, 6:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Use $TARGET_FILE:... instead of get_target_property(... LOCATION)
 
 This means CMake no longer warns about Policy CMP0026 is not set when
 building projects that need kde4support
 
 
 Diffs
 -
 
   cmake/modules/ECMQt4To5Porting.cmake 
 4204fa541790aa38c74b9d6f0b2111af2157b2bc 
   cmake/modules/KDE4Macros.cmake 192094b1c5c6877cd9dfa18dc27338c491d753f3 
   src/CMakeLists.txt 6bda0996c118ce212860e02d0505fd3bdc026833 
   src/KDEUIMacros.cmake 1570df340a672a597a0b7480885c340faca0069d 
 
 Diff: https://git.reviewboard.kde.org/r/116090/diff/
 
 
 Testing
 ---
 
 kde4support compiles, kde-workspace compiles
 
 
 Thanks,
 
 Alexander Richardson
 


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


Re: Review Request 116090: Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

2014-02-27 Thread Alex Merry

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


The problem with doing this in support code is that it is not strictly source 
compatible.  An example this would break is if you want to embed the value of 
QT_QMAKE_EXECUTABLE into a C++ executable using something like
add_definitions(-DQMAKE=\${QT_QMAKE_EXECUTABLE}\)
Any use of QMAKE in the program would then expand to 
$TARGET_FILE:Qt5::qmake, which is obviously not what was wanted.

- Alex Merry


On Feb. 26, 2014, 5:03 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116090/
 ---
 
 (Updated Feb. 26, 2014, 5:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Use $TARGET_FILE:... instead of get_target_property(... LOCATION)
 
 This means CMake no longer warns about Policy CMP0026 is not set when
 building projects that need kde4support
 
 
 Diffs
 -
 
   cmake/modules/ECMQt4To5Porting.cmake 
 4204fa541790aa38c74b9d6f0b2111af2157b2bc 
   cmake/modules/KDE4Macros.cmake 192094b1c5c6877cd9dfa18dc27338c491d753f3 
   src/CMakeLists.txt 6bda0996c118ce212860e02d0505fd3bdc026833 
   src/KDEUIMacros.cmake 1570df340a672a597a0b7480885c340faca0069d 
 
 Diff: https://git.reviewboard.kde.org/r/116090/diff/
 
 
 Testing
 ---
 
 kde4support compiles, kde-workspace compiles
 
 
 Thanks,
 
 Alexander Richardson
 


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


Review Request 116090: Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

2014-02-26 Thread Alexander Richardson

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

Review request for KDE Frameworks.


Repository: kde4support


Description
---

Use $TARGET_FILE:... instead of get_target_property(... LOCATION)

This means CMake no longer warns about Policy CMP0026 is not set when
building projects that need kde4support


Diffs
-

  cmake/modules/ECMQt4To5Porting.cmake 4204fa541790aa38c74b9d6f0b2111af2157b2bc 
  cmake/modules/KDE4Macros.cmake 192094b1c5c6877cd9dfa18dc27338c491d753f3 
  src/CMakeLists.txt 6bda0996c118ce212860e02d0505fd3bdc026833 
  src/KDEUIMacros.cmake 1570df340a672a597a0b7480885c340faca0069d 

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


Testing
---

kde4support compiles, kde-workspace compiles


Thanks,

Alexander Richardson

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