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

[Thomas Lübking]

> On Sept. 19, 2014, 8:24 nachm., 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).

"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)


- Thomas


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


On Sept. 19, 2014, 8:43 nachm., 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, 8:43 nachm.)
> 
> 
> 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-core-devel/attachments/20140919/8bcf83b9/attachment.htm>