Re: Review Request 117012: Place KJsEmbed camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed

2014-04-14 Thread Kevin Ottens 

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


Any news about that patch?

- Kevin Ottens


On March 24, 2014, 1:33 a.m., Andrius da Costa Ribas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117012/
 ---
 
 (Updated March 24, 2014, 1:33 a.m.)
 
 
 Review request for KDE Frameworks and kdewin.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 Currently kjsembed CMake file tries to install both 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not 
 allowed in a case-insensitive filesystem, causing the install step to fail on 
 Windows.
 
 
 Diffs
 -
 
   src/kjsembed/CMakeLists.txt e0ab74c 
 
 Diff: https://git.reviewboard.kde.org/r/117012/diff/
 
 
 Testing
 ---
 
 Tested using MSVC 2013
 
 
 Thanks,
 
 Andrius da Costa Ribas
 


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

Re: Review Request 117012: Place KJsEmbed camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed

2014-03-31 Thread Kevin Ottens 


 On March 27, 2014, 3:47 p.m., Kevin Ottens wrote:
  src/kjsembed/CMakeLists.txt, line 75
  https://git.reviewboard.kde.org/r/117012/diff/1/?file=256496#file256496line75
 
  Agreed, that should be the preferred way.
  
  Note that some other frameworks probably carry the same mistake
 
 Andrius da Costa Ribas wrote:
 I'll update the patch as per Cristophe's and Aleix's comments.
 
 From what I can see from the frameworks I've built on Windows at the 
 moment (all tier1 and most tier2), most of them has 
 include/KF5/FrameworkName/headers, except:
 Attica (include/KF5/Attica/Attica/headers)
 KDNSSD (include/KF5/KDNSSD/DNSSD/headers)
 Solid (include/KF5/Solid/Solid/headers)
 ThreadWeaver (include/KF5/ThreadWeaver/ThreadWeaver/headers)
 *Sonnet (include/Kf5/SonnetCore/Sonnet/headers and 
 include/Kf5/SonnetUi/Sonnet/headers)
 *KDocTools (include/KF5/XsltKde/headers)
 
 Can we consider fixing those too? (I'm just not sure about the ones I 
 marked with *)


No, that's actually fine. We have a different convention for frameworks having 
K* classes and frameworks which are namespaced (like solid, threadweaver, etc.)


- Kevin


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


On March 24, 2014, 1:33 a.m., Andrius da Costa Ribas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117012/
 ---
 
 (Updated March 24, 2014, 1:33 a.m.)
 
 
 Review request for KDE Frameworks and kdewin.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 Currently kjsembed CMake file tries to install both 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not 
 allowed in a case-insensitive filesystem, causing the install step to fail on 
 Windows.
 
 
 Diffs
 -
 
   src/kjsembed/CMakeLists.txt e0ab74c 
 
 Diff: https://git.reviewboard.kde.org/r/117012/diff/
 
 
 Testing
 ---
 
 Tested using MSVC 2013
 
 
 Thanks,
 
 Andrius da Costa Ribas
 


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

Re: Review Request 117012: Place KJsEmbed camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed

2014-03-30 Thread Andrius da Costa Ribas 


 On March 27, 2014, 3:47 p.m., Kevin Ottens wrote:
  src/kjsembed/CMakeLists.txt, line 75
  https://git.reviewboard.kde.org/r/117012/diff/1/?file=256496#file256496line75
 
  Agreed, that should be the preferred way.
  
  Note that some other frameworks probably carry the same mistake

I'll update the patch as per Cristophe's and Aleix's comments.

From what I can see from the frameworks I've built on Windows at the moment 
(all tier1 and most tier2), most of them has 
include/KF5/FrameworkName/headers, except:
Attica (include/KF5/Attica/Attica/headers)
KDNSSD (include/KF5/KDNSSD/DNSSD/headers)
Solid (include/KF5/Solid/Solid/headers)
ThreadWeaver (include/KF5/ThreadWeaver/ThreadWeaver/headers)
*Sonnet (include/Kf5/SonnetCore/Sonnet/headers and 
include/Kf5/SonnetUi/Sonnet/headers)
*KDocTools (include/KF5/XsltKde/headers)

Can we consider fixing those too? (I'm just not sure about the ones I marked 
with *)


- Andrius da Costa


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


On March 24, 2014, 1:33 a.m., Andrius da Costa Ribas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117012/
 ---
 
 (Updated March 24, 2014, 1:33 a.m.)
 
 
 Review request for KDE Frameworks and kdewin.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 Currently kjsembed CMake file tries to install both 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not 
 allowed in a case-insensitive filesystem, causing the install step to fail on 
 Windows.
 
 
 Diffs
 -
 
   src/kjsembed/CMakeLists.txt e0ab74c 
 
 Diff: https://git.reviewboard.kde.org/r/117012/diff/
 
 
 Testing
 ---
 
 Tested using MSVC 2013
 
 
 Thanks,
 
 Andrius da Costa Ribas
 


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

Re: Review Request 117012: Place KJsEmbed camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed

2014-03-27 Thread Kevin Ottens 

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



src/kjsembed/CMakeLists.txt
https://git.reviewboard.kde.org/r/117012/#comment37992

Agreed, that should be the preferred way.

Note that some other frameworks probably carry the same mistake


- Kevin Ottens


On March 24, 2014, 1:33 a.m., Andrius da Costa Ribas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117012/
 ---
 
 (Updated March 24, 2014, 1:33 a.m.)
 
 
 Review request for KDE Frameworks and kdewin.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 Currently kjsembed CMake file tries to install both 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not 
 allowed in a case-insensitive filesystem, causing the install step to fail on 
 Windows.
 
 
 Diffs
 -
 
   src/kjsembed/CMakeLists.txt e0ab74c 
 
 Diff: https://git.reviewboard.kde.org/r/117012/diff/
 
 
 Testing
 ---
 
 Tested using MSVC 2013
 
 
 Thanks,
 
 Andrius da Costa Ribas
 


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

Re: Review Request 117012: Place KJsEmbed camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed

2014-03-24 Thread Aleix Pol Gonzalez 

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


Then you should modify the target_include_directories INSTALL_INTERFACE data, 
no?

- Aleix Pol Gonzalez


On March 24, 2014, 1:33 a.m., Andrius da Costa Ribas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117012/
 ---
 
 (Updated March 24, 2014, 1:33 a.m.)
 
 
 Review request for KDE Frameworks and kdewin.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 Currently kjsembed CMake file tries to install both 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and 
 ${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not 
 allowed in a case-insensitive filesystem, causing the install step to fail on 
 Windows.
 
 
 Diffs
 -
 
   src/kjsembed/CMakeLists.txt e0ab74c 
 
 Diff: https://git.reviewboard.kde.org/r/117012/diff/
 
 
 Testing
 ---
 
 Tested using MSVC 2013
 
 
 Thanks,
 
 Andrius da Costa Ribas
 


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

Review Request 117012: Place KJsEmbed camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed

2014-03-23 Thread Andrius da Costa Ribas 

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

Review request for KDE Frameworks and kdewin.


Repository: kjsembed


Description
---

Currently kjsembed CMake file tries to install both 
${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and 
${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not 
allowed in a case-insensitive filesystem, causing the install step to fail on 
Windows.


Diffs
-

  src/kjsembed/CMakeLists.txt e0ab74c 

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


Testing
---

Tested using MSVC 2013


Thanks,

Andrius da Costa Ribas

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