Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated May 23, 2016, 10:23 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review95726 --- I've tweaked defaults provided by the theme plugin a bit

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 2:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 2:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. > > René J.V. Bertin wrote: > I quite like the way things are, but yes, I've played with that idea. > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:36 p.m., Martin Gräßlin wrote: > > > I saw that one of the justifications for the move was a dependency on > > > Plasma library/ies, but I don't see any such dependencies in the latest > > > version before the move (commit 07548ac1fe7a7fb31a941473911e982fb623c07d). > > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:37 p.m., Martin Gräßlin wrote: > > given that Plasma integration got removed I suggest to create a dedicated > > osx-integration repository for it as well. I quite like the way things are, but yes, I've played with that idea. I guess I can simply fork

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On May 17, 2016, 4:36 p.m., Martin Gräßlin wrote: > > > I saw that one of the justifications for the move was a dependency on > > > Plasma library/ies, but I don't see any such dependencies in the latest > > > version before the move (commit 07548ac1fe7a7fb31a941473911e982fb623c07d). > > >

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review95538 --- given that Plasma integration got removed I suggest to

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review95537 --- > I saw that one of the justifications for the move was a

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated May 16, 2016, 9:49 p.m.) Review request for KDE Software on Mac

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated May 16, 2016, 3:21 p.m.) Review request for KDE Software on Mac

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 18, 2015, 8:11 a.m., Martin Gräßlin wrote: > > I still want to see other OSX devs comment on that this is really the > > wanted approach. Also I need to point out that the split out of QPT to > > plasma-integration is prepared. I suggest to introduce a comparable > > osx-integration

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 18, 2015, 8:11 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.cpp, lines 45-46 > > > > > > Q_NULLPTR > > René J.V. Bertin wrote: > I recall a relatively recent discussion on a

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 18, 2015, 8:11 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.cpp, lines 45-46 > > > > > > Q_NULLPTR > > René J.V. Bertin wrote: > I recall a relatively recent discussion on a

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 18, 2015, 5:53 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 18, 2015, 8:11 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.cpp, lines 45-46 > > > > > > Q_NULLPTR > > René J.V. Bertin wrote: > I recall a relatively recent discussion on a

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 17, 2015, 7:40 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89684 --- I still want to see other OSX devs comment on that this is

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 14, 2015, 7:56 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.cpp, line 45 > > > > > > this looks like a not needed debug statement, same in dtor and > > loadSettings and

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 14, 2015, 7:56 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.cpp, line 45 > > > > > > this looks like a not needed debug statement, same in dtor and > > loadSettings and

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89448 --- src/platformtheme/kdeplatformtheme.cpp (line 45)

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 7, 2015, 5:41 p.m., René J.V. Bertin wrote: > > So with the current implementation that uses inheritance, there are 2 > > classes that have a `delayedDBusConnects` method which uses > > `QDBusConnection::sessionBus().connect()` to register things on the DBus. > > Both invoke that

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 12, 2015, 7:08 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89222 --- So with the current implementation that uses inheritance,

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.h, lines 41-54 > > > > > > why did you add the virtual? The methods are marked as Q_DECL_OVERRIDE > > which implies they

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 7, 2015, 5:43 p.m.) Review request for KDE Software on Mac

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.h, lines 41-54 > > > > > > why did you add the virtual? The methods are marked as Q_DECL_OVERRIDE > > which implies they

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote: > > src/platformtheme/main_mac.cpp, line 45 > > > > > > why does the mac platform want to setupXcbFlush? > > René J.V. Bertin wrote: > If it won't ever be

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 4, 2015, 2:05 p.m.) Review request for KDE Software on Mac

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89124 --- src/platformtheme/kdemactheme.h (line 50)

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote: > > Please include Hugo for a review on the KStyle changes. > > > > I'd suggest to split the review into three parts: one about the adjusted > > test (ecm_foo) - that's a no brainer and doesn't need further discussion. > > One about the

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote: > > src/platformtheme/kdeplatformtheme.h, lines 41-54 > > > > > > why did you add the virtual? The methods are marked as Q_DECL_OVERRIDE > > which implies they

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

> On Dec. 4, 2015, 2:40 p.m., Martin Gräßlin wrote: > > No, those we shouldn't need anymore as the toplevel CMake file will reject Qt versions earlier than 5.3.0 . Should I clean up all occurrences from platformtheme, even where they have nothing to do with the Mac version? - René J.V.

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 4, 2015, 7:01 p.m.) Review request for KDE Software on Mac

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89105 --- Please include Hugo for a review on the KStyle changes. I'd

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 3, 2015, 10:51 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote: > > src/platformtheme/kdemactheme.mm, line 39 > > > > > > This makes me very nervous. > > > > Using private APIs is almost always a guarantee the

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote: > > src/platformtheme/kdemactheme.mm, line 39 > > > > > > This makes me very nervous. > > > > Using private APIs is almost always a guarantee the

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 3:15 p.m., Boudhayan Gupta wrote: > > src/platformtheme/kdemactheme.mm, line 39 > > > > > > This makes me very nervous. > > > > Using private APIs is almost always a guarantee the

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 1, 2015, 9:34 p.m., René J.V. Bertin wrote: > > src/platformtheme/kdemactheme.mm, lines 53-87 > > > > > > I thought it would be best to use a native dialog here to show a > > warning dialog, but it

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 8:46 a.m., Martin Gräßlin wrote: > > Overall I think this is now too much code duplication. With this appraoch > > you don't get bug fixes from the base code. I recommed to rather go for > > inheritance to have the actual code which can be shared still together. Yes, I was

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote: > > Just wondering: if your main aim is to change default settings, why don't > > follow what linux distros do? That is ship some default configuration > > interface. For inspiration check e.g. kubuntu-default-settings. > > René J.V. Bertin

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote: > > Just wondering: if your main aim is to change default settings, why don't > > follow what linux distros do? That is ship some default configuration > > interface. For inspiration check e.g. kubuntu-default-settings. > > René J.V. Bertin

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote: > > Just wondering: if your main aim is to change default settings, why don't > > follow what linux distros do? That is ship some default configuration > > interface. For inspiration check e.g. kubuntu-default-settings. If I understand you

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 2, 2015, 10:38 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 2, 2015, 10:42 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 3:15 p.m., Boudhayan Gupta wrote: > > src/platformtheme/kdemactheme.mm, line 39 > > > > > > This makes me very nervous. > > > > Using private APIs is almost always a guarantee the

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89026 --- src/platformtheme/kdemactheme.mm (line 39)

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dic. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote: > > src/platformtheme/kdemactheme.mm, line 39 > > > > > > This makes me very nervous. > > > > Using private APIs is almost always a guarantee the

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote: > > src/platformtheme/kdemactheme.mm, line 39 > > > > > > This makes me very nervous. > > > > Using private APIs is almost always a guarantee the

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote: > > Just wondering: if your main aim is to change default settings, why don't > > follow what linux distros do? That is ship some default configuration > > interface. For inspiration check e.g. kubuntu-default-settings. > > René J.V. Bertin

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89008 --- A couple of things I jotted down today, probably open doors

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 1, 2015, 9:29 p.m.) Review request for KDE Software on Mac

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89009 --- src/platformtheme/kdemactheme.mm (lines 53 - 87)

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review89021 --- Overall I think this is now too much code duplication. With

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Dec. 1, 2015, 2:03 p.m.) Review request for KDE Software on Mac

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Nov. 30, 2015, 4:51 p.m.) Review request for KDE Software on

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review88970 --- File Attachment: using the KDEPlatformTheme - Screen Shot

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/#review88960 --- I'm strictly against OSX specific changes in framework

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

> On Nov. 30, 2015, 12:42 p.m., Martin Gräßlin wrote: > > I'm strictly against OSX specific changes in framework integration. In my > > opinion framework integration should only be about Plasma. I'll start a new > > thread about it. I was afraid you'd be saying that. I can only hope that

Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- Review request for KDE Software on Mac OS X and KDE Frameworks.

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126198/ --- (Updated Nov. 29, 2015, 8:53 p.m.) Review request for KDE Software on