Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-06 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/ --- Review request for KDE Frameworks and Plasma. Description --- Add KCo

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-06 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review37219 --- As a non-Plasma user of this class, I fully support it being he

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-06 Thread Martin Gräßlin
> On Aug. 6, 2013, 4:52 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 112 > > > > > > This looks like it should be const > > > > I suspect it wasn't because KCoreConfigSelet

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-06 Thread Aleix Pol Gonzalez
> On Aug. 6, 2013, 2:52 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 112 > > > > > > This looks like it should be const > > > > I suspect it wasn't because KCoreConfigSelet

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-06 Thread Albert Astals Cid
> On Aug. 6, 2013, 2:52 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 112 > > > > > > This looks like it should be const > > > > I suspect it wasn't because KCoreConfigSelet

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-06 Thread Martin Gräßlin
> On Aug. 6, 2013, 4:52 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 112 > > > > > > This looks like it should be const > > > > I suspect it wasn't because KCoreConfigSelet

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-06 Thread Martin Gräßlin
> On Aug. 6, 2013, 4:52 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 112 > > > > > > This looks like it should be const > > > > I suspect it wasn't because KCoreConfigSelet

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-07 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review37284 --- tier1/kconfig/autotests/kconfigloadertest.cpp

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-07 Thread Martin Gräßlin
> On Aug. 7, 2013, 5:41 p.m., David Faure wrote: > > tier1/kconfig/autotests/kconfigloadertest.cpp, line 56 > > > > > > I have trouble understanding the purpose of this class. How is this > > different from > >

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-07 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/ --- (Updated Aug. 8, 2013, 6:58 a.m.) Review request for KDE Frameworks, Plasm

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-08 Thread David Faure
> On Aug. 7, 2013, 3:41 p.m., David Faure wrote: > > tier1/kconfig/autotests/kconfigloadertest.cpp, line 56 > > > > > > I have trouble understanding the purpose of this class. How is this > > different from > >

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-08 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/ --- (Updated Aug. 8, 2013, 2:56 p.m.) Review request for KDE Frameworks, Plasm

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-08 Thread Kevin Ottens
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review37360 --- Totally makes sense to have that in KConfigGui. Still the issue

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-08 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/ --- (Updated Aug. 9, 2013, 7:20 a.m.) Review request for KDE Frameworks, Plasm

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-09 Thread Kevin Ottens
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review37410 --- tier1/kconfig/src/gui/kconfigloader.h

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-09 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/ --- (Updated Aug. 9, 2013, 2:38 p.m.) Review request for KDE Frameworks, Plasm

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-09 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review37413 --- tier1/kconfig/src/gui/kconfigloader.cpp

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-09 Thread Kai Uwe Broulik
> On Aug. 9, 2013, 2:55 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.cpp, line 65 > > > > > > This would be best with use of QLatin1String() thoughout this file. > > > > QStringLitera

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-09 Thread David Edmundson
> On Aug. 9, 2013, 2:55 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.cpp, line 65 > > > > > > This would be best with use of QLatin1String() thoughout this file. > > > > > > Kai Uwe B

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-09 Thread David Edmundson
> On Aug. 9, 2013, 2:55 p.m., David Edmundson wrote: > > tier1/kconfig/src/gui/kconfigloader.cpp, line 65 > > > > > > This would be best with use of QLatin1String() thoughout this file. > > > > > > Kai Uwe B

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-08-09 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review37445 --- tier1/kconfig/src/gui/kconfigloader.h

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-09-02 Thread Martin Gräßlin
> On Aug. 9, 2013, 9:27 p.m., David Faure wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 99 > > > > > > I wonder if this should really derive from KConfigSkeleton, rather than > > encapsulate it. > >

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-09-03 Thread David Faure
> On Aug. 9, 2013, 7:27 p.m., David Faure wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 99 > > > > > > I wonder if this should really derive from KConfigSkeleton, rather than > > encapsulate it. > >

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-09-11 Thread Martin Gräßlin
> On Aug. 9, 2013, 9:27 p.m., David Faure wrote: > > tier1/kconfig/src/gui/kconfigloader.h, line 99 > > > > > > I wonder if this should really derive from KConfigSkeleton, rather than > > encapsulate it. > >

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-09-12 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review39867 --- Ship it! OK then. - David Faure On Aug. 9, 2013, 12:38 p.m.

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-09-16 Thread Commit Hook
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/#review40091 --- This review has been submitted with commit 5bd41f0bb007569af3e

Re: Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

2013-09-16 Thread Commit Hook
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111908/ --- (Updated Sept. 16, 2013, 8:05 a.m.) Status -- This change has been ma