> On Nov. 15, 2014, 9:41 a.m., Martin Gräßlin wrote:
> > kdeui/tests/kwindowtest.cpp, lines 218-239
> > <https://git.reviewboard.kde.org/r/120931/diff/2/?file=328515#file328515line218>
> >
> >     please note that your test application is fragile as it's based on 
> > sleep which is dangerous on an async windowing system.
> >     
> >     If you want to perform proper tests have a look at the auto-tests in 
> > KWindowSystem framework. I would be very pleased if it gets an OSX port and 
> > also the auto-tests adjusted.

Concerning KF5: there is currently only 1 person who could check if these 
modifications *compile* and possibly if a well-written test succeeds - Marko.
It is my understanding that improving existing features (like OS X support) 
falls in the category of bug fixes according to Albert Astals's definitions. If 
this gets committed to an OS X specific branch that's fine with me too.

Considering the test: I don't think the test application does async things and 
I'm not even sure if it really has an event loop. But you're right, using sleep 
is a very quick hack that I ought to have replaced by something using QTimer or 
equivalent. Note however that this is an addition to an existing test case from 
KWindowSystem; where are the auto-tests that you mention?


- René J.V.


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


On Nov. 15, 2014, 12:04 a.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120931/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2014, 12:04 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This is an attempt to improve the Mac-specific implementation of the 
> `KWindowSystem` class.
> For convenience and future-proofness (and also because I like the language) I 
> converted `kwindowsystem_mac.cpp` to ObjC++, i.e. `kwindowsystem_mac.mm`, and 
> added the AppKit framework in the CMakeFile.
> 
> Much of the code in this file is hardly better than gentle hacking, but that 
> probably concerns the functions that are of least interest on a platform 
> where KDE doesn't do session management.
> 
> I should probably update the "not yet implemented" debug statements (to 
> "unsupported"), and I might have another look at kwindowinfo_mac.cpp too.
> 
> 
> Diffs
> -----
> 
>   kdeui/CMakeLists.txt 1454790 
>   kdeui/tests/kwindowtest.cpp b4012d7 
>   kdeui/windowmanagement/kwindowsystem_mac.cpp 4200237 
>   kdeui/windowmanagement/kwindowsystem_mac_p.h PRE-CREATION 
>   kdeui/windowmanagement/kwindowsystem_macobjc.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/120931/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.6.8, mostly with the updated kwindowtest utility (which calls 
> KWindowSystem functions when clicking the Open button in its toolbar).
> Also tested on Mac OS X 10.9.4 rebuilding kdelibs from scratch.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Reply via email to