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

[Marko Käning]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313626#file313626line225>
> >
> >     tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
>     I looked into those yesterday, and cannot find tabs on my end. No idea where they'd come from.
> 
> Martin Gräßlin wrote:
>     I'd guess the editor you use changes it somehow. I'd suggest to use Kate and enable show all whitespaces.
> 
> René J.V. Bertin wrote:
>     No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in vi (which doesn't modify anything unless I ask for it) and there are no tabs to be found. Same in KDevelop when I open the patch review tool. I wonder if your editor isn't the culprit, or something on reviewboard?
>     
>     I copied the .reviewboardrc file from another KDE project and edited the obvious settings. I see nothing concerning tabs in there, but maybe there ought to be?
> 
> René J.V. Bertin wrote:
>     See my local copy of the diff I just added.
> 
> Thomas Lübking wrote:
>     There're no tabs in the diff (checked teh only reliable way: okteta ;-)
>     
>     Indention in those files is crazy (2 or 3 chars, never 4), what makes it look like WS/tab intermix, but ">>>" seems only a RB indication that the line was altered only by shifting.
> 
> René J.V. Bertin wrote:
>     Just a question: would I still have triggered `-pedantic` mode if I had included something like "and relevant code tidy-ups" in my RR title? :P
>     
>     Also, is one supposed to go through an RR for each and every tidy-up one might want to commit (now that I have commit access ...)?
> 
> Thomas Lübking wrote:
>     You can stuff as many patches (commits) into one review as you like, just that one patch may delay the "ship it" to all others than and at some point ppl. will refuse to look at 3MB of patches at once ;-)
>     
>     I don't think that pure tidy up commits belong into KDE4 at all, if you really consider one necessary, better have it reviewed.
>     
>     For KF5/master you ideally clean up code while editing the area anyway.
>     Global astyle fixes should only be done when things become too much of a mess - and be shown to the maintainer.
>     (Reason is that you introduce "fences" to "git blame")

Reason being that "fences" to "git blame" get introduced is a very good point, indeed! (Never thought of that, actually.)
That's somewhat a problem of the SCM's way of handling the code lines then!
Would be cool if white-space commits could be marked as such, so that git could omit/bridge them in case of blame...


- Marko


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


On Sept. 20, 2014, 12:54 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. 20, 2014, 12:54 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).
> 
> 
> File Attachments
> ----------------
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140920/7d226085/attachment.html>