> 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") > > Marko Käning wrote: > 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... > > Christoph Feck wrote: > The >>> markers reviewboard shows are not Tabs, but change in indentation.
> would I still have triggered -pedantic mode if I had included something like > "and relevant code tidy-ups" in my RR title? you triggered the -pedantic mode for me (which is always kind of on), because I consider the change as dangerous, thus I think it must be absolutely side-effect free and in a way that reviewers can focus on the actual changes. Sorry about the whitespace complains, that must be a recent change in reviewboard - at least I have never seen it before. - Martin ----------------------------------------------------------- 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 > >
