Re: Review Request 109551: port KPtyProcess to QProcess

2013-03-19 Thread Oswald Buddenhagen 


 On March 18, 2013, 10:04 p.m., Oswald Buddenhagen wrote:
  kpty/tests/kptyprocesstest.cpp, line 193
  http://git.reviewboard.kde.org/r/109551/diff/2/?file=120310#file120310line193
 
  i don't think eating the sleep is a good idea. i'm sure i added it for 
  a reason (in a previous life ^^).
 
 Martin Tobias Holmedahl Sandsmark wrote:
 with the sleep the test fails, and even with high load I can't get the 
 test to fail without it.

hmm, that means that something is actually broken in the port ... i'll need to 
look at it a bit more thoroughly. don't expect this to happen before the 
weekend.


 On March 18, 2013, 10:04 p.m., Oswald Buddenhagen wrote:
  kpty/tests/kptyprocesstest.cpp, line 210
  http://git.reviewboard.kde.org/r/109551/diff/2/?file=120310#file120310line210
 
  the -c needs to be a separate argument.
  
  the quotes, backslashes and attempt at a newline are all garbage.
 
 Martin Tobias Holmedahl Sandsmark wrote:
 I first had -c as a separate argument, and it didn't matter. And I had to 
 add an extra newline to get it to work with doing it manually.

well, i'm just telling you how it should be, i.e., what setShellCommand() did. 
if that doesn't work, we have a real problem - most likely related to the other 
failure.


- Oswald


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109551/#review29478
---


On March 18, 2013, 7:54 p.m., Martin Tobias Holmedahl Sandsmark wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/109551/
 ---
 
 (Updated March 18, 2013, 7:54 p.m.)
 
 
 Review request for KDE Frameworks, kdelibs, David Faure, and Oswald 
 Buddenhagen.
 
 
 Description
 ---
 
 Just a simple port of KPtyProcess away from using KProcess.
 
 
 Diffs
 -
 
   kpty/kptyprocess.h 5e0df96 
   kpty/kptyprocess.cpp 015a58c 
   kpty/tests/kptyprocesstest.cpp 04990a0 
 
 Diff: http://git.reviewboard.kde.org/r/109551/diff/
 
 
 Testing
 ---
 
 builds and tests pass.
 
 
 Thanks,
 
 Martin Tobias Holmedahl Sandsmark
 


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

Re: Review Request 109551: port KPtyProcess to QProcess

2013-03-19 Thread Jan Kundrát 

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109551/#review29432
---



kpty/tests/kptyprocesstest.h
http://git.reviewboard.kde.org/r/109551/#comment21974

Why are these commented out instead of being removed?


- Jan Kundrát


On March 17, 2013, 4:44 p.m., Martin Tobias Holmedahl Sandsmark wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/109551/
 ---
 
 (Updated March 17, 2013, 4:44 p.m.)
 
 
 Review request for KDE Frameworks, kdelibs and David Faure.
 
 
 Description
 ---
 
 Just a simple port of KPtyProcess away from using KProcess.
 
 
 Diffs
 -
 
   kpty/kptyprocess.h 5e0df96 
   kpty/kptyprocess.cpp 015a58c 
   kpty/tests/kptyprocesstest.h 64bded0 
   kpty/tests/kptyprocesstest.cpp 04990a0 
 
 Diff: http://git.reviewboard.kde.org/r/109551/diff/
 
 
 Testing
 ---
 
 builds and tests pass.
 
 
 Thanks,
 
 Martin Tobias Holmedahl Sandsmark
 


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

Re: [cmake-developers] CMake usage requirements in KDE Frameworks

2013-03-19 Thread Brad King 
On 03/19/2013 04:02 PM, Alexander Neundorf wrote:
 On Tuesday 19 March 2013, Stephen Kelly wrote:
 I don't mind renaming it, but it's a topic for the cmake list.
 
 So, I think the variable name CMAKE_BUILD_INTERFACE_INCLUDES is to general.
 Since it is similar to the effect of CMAKE_INCLUDE_CURRENT_DIR, I suggest 
 renaming it e.g. to CMAKE_INCLUDE_CURRENT_DIR_IN_INTERFACE.

The proposed name is much better.  The old name was never in a final
release so it is easy to rename now:

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=fc43477d

 applies to both the BUILD and INSTALL include interfaces. To limit it to
 one or the other, you have to specify it:

  target_include_directories(foo INTERFACE
$BUILD_INTERFACE:/foo/interface/only
  )
 
 Why was this generator expression approach chosen over simply adding suitable 
 keywords, as it is done for all other cmake commands ?
 
 target_include_directories(foo INTERFACE_BUILD ${CMAKE_CURRENT_SOURCE_DIR} 

 ${CMAKE_CURRENT_SOURCE_DIR}/whatever/
${CMAKE_CURRENT_BINARY_DIR}
INTERFACE_INSTALL ${INCLUDE_INSTALL_DIR} ) 

The keywords won't interact well with PUBLIC/PRIVATE/INTERFACE keywords.
Also the arguments are likely to be built up in lists:

  target_include_directories(foo PUBLIC
 ${foo_INCLUDE_DIRS} ${other_INCLUDE_DIRS}
)

that may or may not contain multiple entries and multiple uses of each
$BUILD_INTERFACE:... and $INSTALL_INTERFACE:... expression.
The generator expressions bind tightly.

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