Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-31 Thread Marco Martin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/#review63555 --- Ship it! I kinda liked the singlethon as well, but seems

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-31 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/#review63563 --- Ship it! Good to me too.

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-31 Thread Sebastian Kügler
On July 31, 2014, 12:56 p.m., David Edmundson wrote: src/qmlcontrols/kcoreaddons/kuserproxy.cpp, line 128 https://git.reviewboard.kde.org/r/119535/diff/2/?file=294403#file294403line128 This is a static method. You don't need to call QHostInfo ctor. Hah, thanks for catching it. I

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-31 Thread Sebastian Kügler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/ --- (Updated July 31, 2014, 1:15 p.m.) Status -- This change has been

Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Sebastian Kügler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/ --- Review request for Plasma. Repository: kdeclarative Description

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Sebastian Kügler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/ --- (Updated July 29, 2014, 2:31 p.m.) Review request for KDE Frameworks and

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/#review63441 --- Can we consider including this in KCoreAddons? I know

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/#review63443 --- Looks damn good, especially the documentation and example.

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Marco Martin
On July 29, 2014, 2:33 p.m., Aleix Pol Gonzalez wrote: Can we consider including this in KCoreAddons? I know KCoreAddons should only depend on QtCore, but then we can make it an optional dependency. If Qt5::Qml is there, then build the Qml plugin too. KCoreAddons can depend from

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Bhushan Shah
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/#review63453 --- src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread David Edmundson
On July 29, 2014, 2:33 p.m., Aleix Pol Gonzalez wrote: Can we consider including this in KCoreAddons? I know KCoreAddons should only depend on QtCore, but then we can make it an optional dependency. If Qt5::Qml is there, then build the Qml plugin too. Marco Martin wrote:

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Aleix Pol Gonzalez
On July 29, 2014, 4:34 p.m., Bhushan Shah wrote: src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp, line 44 https://git.reviewboard.kde.org/r/119535/diff/1/?file=294241#file294241line44 Maybe consider creating it a singleton type? Why? In fact, I think it's usually harder to deal

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Marco Martin
On July 29, 2014, 2:33 p.m., Aleix Pol Gonzalez wrote: Can we consider including this in KCoreAddons? I know KCoreAddons should only depend on QtCore, but then we can make it an optional dependency. If Qt5::Qml is there, then build the Qml plugin too. Marco Martin wrote:

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Bhushan Shah
On July 29, 2014, 10:04 p.m., Bhushan Shah wrote: src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp, line 44 https://git.reviewboard.kde.org/r/119535/diff/1/?file=294241#file294241line44 Maybe consider creating it a singleton type? Aleix Pol Gonzalez wrote: Why? In fact, I

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Bhushan Shah
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/#review63459 --- src/qmlcontrols/kcoreaddons/kuserproxy.h

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Sebastian Kügler
On July 29, 2014, 4:53 p.m., Bhushan Shah wrote: src/qmlcontrols/kcoreaddons/kuserproxy.h, line 96 https://git.reviewboard.kde.org/r/119535/diff/1/?file=294242#file294242line96 why? nameChanged is not related here.. It's not visible API (the signal visible to QML has the name of

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Sebastian Kügler
On July 29, 2014, 4:34 p.m., Bhushan Shah wrote: src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp, line 44 https://git.reviewboard.kde.org/r/119535/diff/1/?file=294241#file294241line44 Maybe consider creating it a singleton type? Aleix Pol Gonzalez wrote: Why? In fact, I

Re: Review Request 119535: Move QML bindings for KUser to kdeclarative

2014-07-29 Thread Sebastian Kügler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119535/ --- (Updated July 29, 2014, 6:04 p.m.) Review request for KDE Frameworks and