subject:"Review Request 117012\: Place KJsEmbed headers directly under $\{INCLUDE_INSTALL_DIR\}\/KJsEmbed instead of under $\{INCLUDE_INSTALL_DIR\}\/KJsEmbed\/kjsembed \[was\: Place KJsEmbed camelcase header under $"

Re: Review Request 117012: Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

2014-04-26 Thread Aleix Pol Gonzalez



 On April 23, 2014, 5:09 p.m., David Faure wrote:
  The reason I did it this way was because existing application code uses
  
  #include kjsembed/kjsembed.h
  #include kjsembed/qobject_binding.h
  etc.
  See e.g. 
  http://lxr.kde.org/source/kde/kdelibs/kross/kjs/kjsscript.cpp?v=stable-qt4
  
  So I picked the namespaced framework setup, where lowercase headers go to 
  include/KF5/FrameworkName/lower/lower.h and forwarding headers go to 
  include/KF5/FrameworkName/Camelcase/Camelcase. KParts does the same, for 
  instance, with e.g. include/KF5/KParts/kparts/event.h and 
  include/KF5/KParts/KParts/Event.
  
  So why is this a problem for kjsembed and not for kparts? Ah, I see. The 
  forwarding header is supposed to go under a camelcase subdir (like it does 
  in kparts). KParts/KParts/Event looks confusing, but a better example where 
  the framework name differs from the namespace is KIOCore/KIO/Job. 
  
  So this should be the fix you need:
  
  -install(FILES ${KJsEmbed_CamelCase_HEADERS} DESTINATION 
  ${INCLUDE_INSTALL_DIR}/KJsEmbed COMPONENT Devel)
  +install(FILES ${KJsEmbed_CamelCase_HEADERS} DESTINATION 
  ${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed COMPONENT Devel)
 

+1, this would make it work the same as KParts.

Resulting in include/KF5/KJsEmbed/KJsEmbed/KJsEmbed. Do we have projects 
including only KJsEmbed? In this case we'd have to introduce an interface for 
KJsEmbed/KJsEmbed.


- Aleix


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


On April 21, 2014, 1:59 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 April 21, 2014, 1:59 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 headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

2014-04-26 Thread David Faure



 On April 23, 2014, 5:09 p.m., David Faure wrote:
  The reason I did it this way was because existing application code uses
  
  #include kjsembed/kjsembed.h
  #include kjsembed/qobject_binding.h
  etc.
  See e.g. 
  http://lxr.kde.org/source/kde/kdelibs/kross/kjs/kjsscript.cpp?v=stable-qt4
  
  So I picked the namespaced framework setup, where lowercase headers go to 
  include/KF5/FrameworkName/lower/lower.h and forwarding headers go to 
  include/KF5/FrameworkName/Camelcase/Camelcase. KParts does the same, for 
  instance, with e.g. include/KF5/KParts/kparts/event.h and 
  include/KF5/KParts/KParts/Event.
  
  So why is this a problem for kjsembed and not for kparts? Ah, I see. The 
  forwarding header is supposed to go under a camelcase subdir (like it does 
  in kparts). KParts/KParts/Event looks confusing, but a better example where 
  the framework name differs from the namespace is KIOCore/KIO/Job. 
  
  So this should be the fix you need:
  
  -install(FILES ${KJsEmbed_CamelCase_HEADERS} DESTINATION 
  ${INCLUDE_INSTALL_DIR}/KJsEmbed COMPONENT Devel)
  +install(FILES ${KJsEmbed_CamelCase_HEADERS} DESTINATION 
  ${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed COMPONENT Devel)
 
 
 Aleix Pol Gonzalez wrote:
 +1, this would make it work the same as KParts.
 
 Resulting in include/KF5/KJsEmbed/KJsEmbed/KJsEmbed. Do we have projects 
 including only KJsEmbed? In this case we'd have to introduce an interface 
 for KJsEmbed/KJsEmbed.

I checked, and I couldn't see any such #include statement.


- David


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


On April 26, 2014, 10:41 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 April 26, 2014, 10:41 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 headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

2014-04-26 Thread Andrius da Costa Ribas


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

(Updated April 26, 2014, 10:41 a.m.)


Status
--

This change has been marked as submitted.


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 headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

2014-04-23 Thread David Faure


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


The reason I did it this way was because existing application code uses

#include kjsembed/kjsembed.h
#include kjsembed/qobject_binding.h
etc.
See e.g. 
http://lxr.kde.org/source/kde/kdelibs/kross/kjs/kjsscript.cpp?v=stable-qt4

So I picked the namespaced framework setup, where lowercase headers go to 
include/KF5/FrameworkName/lower/lower.h and forwarding headers go to 
include/KF5/FrameworkName/Camelcase/Camelcase. KParts does the same, for 
instance, with e.g. include/KF5/KParts/kparts/event.h and 
include/KF5/KParts/KParts/Event.

So why is this a problem for kjsembed and not for kparts? Ah, I see. The 
forwarding header is supposed to go under a camelcase subdir (like it does in 
kparts). KParts/KParts/Event looks confusing, but a better example where the 
framework name differs from the namespace is KIOCore/KIO/Job. 

So this should be the fix you need:

-install(FILES ${KJsEmbed_CamelCase_HEADERS} DESTINATION 
${INCLUDE_INSTALL_DIR}/KJsEmbed COMPONENT Devel)
+install(FILES ${KJsEmbed_CamelCase_HEADERS} DESTINATION 
${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed COMPONENT Devel)


- David Faure


On April 21, 2014, 1:59 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 April 21, 2014, 1:59 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 headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

2014-04-20 Thread Andrius da Costa Ribas


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

(Updated April 21, 2014, 1:59 a.m.)


Review request for KDE Frameworks and kdewin.


Changes
---

place all includes under include/KJsEmbed (instead of having another kjsembed 
subdir) and remove 
$INSTALL_INTERFACE:${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed from 
target_include_directories.


Summary (updated)
-

Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead 
of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed 
camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed]


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 (updated)
-

  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 headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

Re: Review Request 117012: Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

Re: Review Request 117012: Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

Re: Review Request 117012: Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

Re: Review Request 117012: Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

5 matches

Site Navigation

Mail list logo

Footer information