[KDE/Mac] Review Request 120287: [OS X] make kde-workspace build

[René J.V. Bertin]

> On Sept. 19, 2014, 10:24 p.m., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, lines 544-548
> > <https://git.reviewboard.kde.org/r/120287/diff/1/?file=313477#file313477line544>
> >
> >     this looks like an inintended change
> 
> René J.V. Bertin wrote:
>     No, I often do this when conditionals get too nested to see at a glance if you're in an active/relevant block or not.
> 
> Martin Gräßlin wrote:
>     and exactly that's the thing I consider too risky. You changed code which was not relevant for the platform your working on. If you introduced a slight change there and it breaks, nobody would notice (and no, we know that nobody tests our stable build before it goes out to the distros - see my last years talk at Akademy about it).
> 
> Thomas Lübking wrote:
>     "too nested"? "TWO nested" ;-P
>     
>     ```cpp
>     #if FOO
>        foo();
>        #if BAR
>           if (barIsFoo)
>              bar();
>        #else
>           nobar();
>        #fi
>     #fi
>     ```
>     
>     But that's not KDE coding style either.
>     
>     However, I agree with Martin to focus on the required and not introduce unrelated changes, esp. no style "fixes" (of course you can do so during development, just clean the patch afterwards)
>     
>     Other than that I've no major worries that this could introduce regressions outside the Apple world - just please revert the QT_NO_DEBUG_OUTPUT undefinition and the QtDebug include (which I assume ist just a leftover and this *could* have nasty side-effects, eg. when compiling O3)

Yeah, I know, there was no real to change this indentation.

The QT_NO_DEBUG_OUTPUT is indeed a leftover (oops...). 

But how do you compile -O3? Each time I try, the cmake scripts insist on putting a -O2 or -Os further down the commandline...


- René J.V.


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


On Sept. 19, 2014, 10:43 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2014, 10:43 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> A few rather straightforward patches to make the relevant bits of KDE4's kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the various relevant KDE settings among which desktop search, fonts, colours and even style.
> The oxygen style builds and looks good but shows some updating glitches due to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140919/98b13e7a/attachment-0001.html>