----------------------------------------------------------- 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. Minor nitpicks below. src/qmlcontrols/kcoreaddons/kuserproxy.h <https://git.reviewboard.kde.org/r/119535/#comment44218> needs to be LGPL for frameworks. src/qmlcontrols/kcoreaddons/kuserproxy.cpp <https://git.reviewboard.kde.org/r/119535/#comment44219> indentation (could be just reviewboard) src/qmlcontrols/kcoreaddons/kuserproxy.cpp <https://git.reviewboard.kde.org/r/119535/#comment44222> so maybe this should be faceIconUrl? src/qmlcontrols/kcoreaddons/kuserproxy.cpp <https://git.reviewboard.kde.org/r/119535/#comment44220> Not really needed, .close() is called in the QFile destructor and if you want to call it explicitly, you should do it on line 114 too. src/qmlcontrols/kcoreaddons/kuserproxy.cpp <https://git.reviewboard.kde.org/r/119535/#comment44221> There's a Qt method for this. http://qt-project.org/doc/qt-5/qhostinfo.html#localHostName requires QtNetwork, but I imagine we're pulling that in anyway. - David Edmundson On July 29, 2014, 2:31 p.m., Sebastian Kügler wrote: > > ----------------------------------------------------------- > 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 Plasma. > > > Repository: kdeclarative > > > Description > ------- > > Move QML bindings for KUser to kdeclarative > > This code has already been released, but privately inside Kickoff. As it is > not strictly related (or limited to) Kickoff, I'd like to move it into > kdeclarative. There are already other bindings for classes from KCoreAddons > there, so this seems like a nice fit. > > The class is useful as public API since it allows customization of apps, > making the user feel more familiar with the program at hand. I want to use > the name and face icon of the user in more places (systemsettings redesign > comes to mind), so it'd be useful to have it shared. > > > Diffs > ----- > > src/qmlcontrols/kcoreaddons/CMakeLists.txt 597cc2c > src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp 3c1a96e > src/qmlcontrols/kcoreaddons/kuserproxy.h PRE-CREATION > src/qmlcontrols/kcoreaddons/kuserproxy.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/119535/diff/ > > > Testing > ------- > > Ported an (unreleased) app using this class to the new location, works as > expected, no regressions encountered. > > > Thanks, > > Sebastian Kügler > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
