Re: Review Request 115030: Install public headers for KJsEmbed

2014-01-16 Thread Alex Merry

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



src/kjsembed/CMakeLists.txt
https://git.reviewboard.kde.org/r/115030/#comment33765

These don't look like public headers; are they really needed?

Pointer isn't even namespaced.  And kjseglobal.h has the unnamedspaced 
RedirectIOToConsole function.



src/kjsembed/CMakeLists.txt
https://git.reviewboard.kde.org/r/115030/#comment33764

This comment should go.


- Alex Merry


On Jan. 15, 2014, 2:26 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115030/
 ---
 
 (Updated Jan. 15, 2014, 2:26 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 KJsEmbed library is useless without headers installed. This patch installs 
 them.
 
 
 Diffs
 -
 
   CMakeLists.txt fc1731a 
   src/kjsembed/CMakeLists.txt b4d3f5f 
 
 Diff: https://git.reviewboard.kde.org/r/115030/diff/
 
 
 Testing
 ---
 
 I ported the share dataengine to use it (it was using the now defunct Kross 
 KJS support).
 
 https://git.reviewboard.kde.org/r/115027/
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


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


Re: Review Request 115030: Install public headers for KJsEmbed

2014-01-16 Thread Alex Merry


 On Jan. 16, 2014, 1:52 p.m., Alex Merry wrote:
  src/kjsembed/CMakeLists.txt, lines 38-39
  https://git.reviewboard.kde.org/r/115030/diff/1/?file=234081#file234081line38
 
  These don't look like public headers; are they really needed?
  
  Pointer isn't even namespaced.  And kjseglobal.h has the unnamedspaced 
  RedirectIOToConsole function.

Ah, I see, they're used by binding_support.h; I don't think they should be used 
directly, though, and so probably shouldn't have the CamelCase versions.

I'm wary about installing all these headers, and so committing to various SC 
and BC guarantees on them, without cleaning them up first.  The un-namespaced 
Pointer/PointerBase class particularly concerns me.


- Alex


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


On Jan. 15, 2014, 2:26 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115030/
 ---
 
 (Updated Jan. 15, 2014, 2:26 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 KJsEmbed library is useless without headers installed. This patch installs 
 them.
 
 
 Diffs
 -
 
   CMakeLists.txt fc1731a 
   src/kjsembed/CMakeLists.txt b4d3f5f 
 
 Diff: https://git.reviewboard.kde.org/r/115030/diff/
 
 
 Testing
 ---
 
 I ported the share dataengine to use it (it was using the now defunct Kross 
 KJS support).
 
 https://git.reviewboard.kde.org/r/115027/
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


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


Re: Review Request 115030: Install public headers for KJsEmbed

2014-01-16 Thread Alex Merry

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

Ship it!


Still not ideal, but -- in the absence of an enthusiastic maintainer -- that's 
unlikely to change any time soon.  At least this makes it usable, and the names 
shouldn't clash.

- Alex Merry


On Jan. 16, 2014, 2:33 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115030/
 ---
 
 (Updated Jan. 16, 2014, 2:33 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 KJsEmbed library is useless without headers installed. This patch installs 
 them.
 
 
 Diffs
 -
 
   CMakeLists.txt fc1731a 
   src/kjsembed/CMakeLists.txt b4d3f5f 
   src/kjsembed/kjseglobal.h c2b44dc 
   src/kjsembed/pointer.h e25a5e4 
 
 Diff: https://git.reviewboard.kde.org/r/115030/diff/
 
 
 Testing
 ---
 
 I ported the share dataengine to use it (it was using the now defunct Kross 
 KJS support).
 
 https://git.reviewboard.kde.org/r/115027/
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


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


Re: Review Request 115030: Install public headers for KJsEmbed

2014-01-16 Thread Aleix Pol Gonzalez


 On Jan. 16, 2014, 3:48 p.m., Alex Merry wrote:
  Still not ideal, but -- in the absence of an enthusiastic maintainer -- 
  that's unlikely to change any time soon.  At least this makes it usable, 
  and the names shouldn't clash.

:) that's the spirit!

Anyway, without installing headers, the library is useless.


- Aleix


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


On Jan. 16, 2014, 2:33 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115030/
 ---
 
 (Updated Jan. 16, 2014, 2:33 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 KJsEmbed library is useless without headers installed. This patch installs 
 them.
 
 
 Diffs
 -
 
   CMakeLists.txt fc1731a 
   src/kjsembed/CMakeLists.txt b4d3f5f 
   src/kjsembed/kjseglobal.h c2b44dc 
   src/kjsembed/pointer.h e25a5e4 
 
 Diff: https://git.reviewboard.kde.org/r/115030/diff/
 
 
 Testing
 ---
 
 I ported the share dataengine to use it (it was using the now defunct Kross 
 KJS support).
 
 https://git.reviewboard.kde.org/r/115027/
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


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


Re: Review Request 115030: Install public headers for KJsEmbed

2014-01-16 Thread Commit Hook

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


This review has been submitted with commit 
abb9ac7fe0759e1d9f31a838938e8f256272a206 by Aleix Pol to branch master.

- Commit Hook


On Jan. 16, 2014, 2:33 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115030/
 ---
 
 (Updated Jan. 16, 2014, 2:33 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kjsembed
 
 
 Description
 ---
 
 KJsEmbed library is useless without headers installed. This patch installs 
 them.
 
 
 Diffs
 -
 
   CMakeLists.txt fc1731a 
   src/kjsembed/CMakeLists.txt b4d3f5f 
   src/kjsembed/kjseglobal.h c2b44dc 
   src/kjsembed/pointer.h e25a5e4 
 
 Diff: https://git.reviewboard.kde.org/r/115030/diff/
 
 
 Testing
 ---
 
 I ported the share dataengine to use it (it was using the now defunct Kross 
 KJS support).
 
 https://git.reviewboard.kde.org/r/115027/
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


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


Re: Review Request 115030: Install public headers for KJsEmbed

2014-01-16 Thread Aleix Pol Gonzalez

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

(Updated Jan. 16, 2014, 6:30 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kjsembed


Description
---

KJsEmbed library is useless without headers installed. This patch installs them.


Diffs
-

  CMakeLists.txt fc1731a 
  src/kjsembed/CMakeLists.txt b4d3f5f 
  src/kjsembed/kjseglobal.h c2b44dc 
  src/kjsembed/pointer.h e25a5e4 

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


Testing
---

I ported the share dataengine to use it (it was using the now defunct Kross KJS 
support).

https://git.reviewboard.kde.org/r/115027/


Thanks,

Aleix Pol Gonzalez

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


Review Request 115030: Install public headers for KJsEmbed

2014-01-15 Thread Aleix Pol Gonzalez

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

Review request for KDE Frameworks.


Repository: kjsembed


Description
---

KJsEmbed library is useless without headers installed. This patch installs them.


Diffs
-

  CMakeLists.txt fc1731a 
  src/kjsembed/CMakeLists.txt b4d3f5f 

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


Testing
---

I ported the share dataengine to use it (it was using the now defunct Kross KJS 
support).

https://git.reviewboard.kde.org/r/115027/


Thanks,

Aleix Pol Gonzalez

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