D28154: Add users KCM

[Nicolas Fella]

nicolasfella added a comment.


  Couple of nitpicks, mostly coding style and most of that probably comes from me.
  Can't say much otherwise since I wrote quite a bit of it

INLINE COMMENTS

> main.qml:100
> +                Component.onCompleted: () => {
> +                    print("compl")
> +                    print(loggedIn)

remove prints

> kcm.cpp:53
> +
> +    qmlRegisterSingletonType<UserModel>("org.kde.userng", 1, 0, "UserModel", [](QQmlEngine *engine, QJSEngine *scriptEngine) -> QObject* {
> +        Q_UNUSED(engine)

We should probably drop the 'ng'

> kcm.h:27
> +
> +class ConnectionSettings;
> +

copy pasta?

> kcm.h:45
> +
> +#pragma once

remove

> user.cpp:178
> +
> +void User::setPassword(const QString &password) {
> +    m_dbusIface->SetPassword(saltPassword(password), QString());

newline before {, also elsewhere

> usercontroller.cpp:52
> +
> +#include "usercontroller.moc"

Not needed I think

> usermodel.cpp:51
> +    connect(m_dbusInterface, &OrgFreedesktopAccountsInterface::UserDeleted, this, [this](const QDBusObjectPath &path) {
> +        for (User* user: m_userList) {
> +            if (user->path() == path) {

qAsConst(m_userList)

> usermodel.cpp:62
> +
> +    QDBusPendingReply <QList <QDBusObjectPath > > reply = m_dbusInterface->ListCachedUsers();
> +    reply.waitForFinished();

Coding style: no space before '>'

The coding style does not specify this, but IMHO it makes sense to apply the rules for parentheses here

> usermodel.cpp:66
> +    if (reply.isError()) {
> +        qDebug() << reply.error().message();
> +        return;

qCDebug

> usermodel.cpp:72
> +    for (const QDBusObjectPath& path: users) {
> +        qDebug() << "User found";
> +        User *user = new User();

remove or qCDebug

> usermodel.cpp:87
> +    names.insert(FaceRole, "face");
> +    names.insert(AutoLoginRole, "autoLogin");
> +    names.insert(AdministratorRole, "administrator");

If we decide to not have autologin in the KCM then we don't this I guess?

> usermodel.cpp:101
> +{
> +    for (auto user : m_userList) {
> +        if (user->loggedIn()) {

const + qAsConst

> usersessions.h:42
> +    public:
> +        explicit UserSession(QObject* parent = nullptr);
> +        virtual ~UserSession();

Coding style: *parent
Also elsewhere

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28154

To: cblack, #plasma, #vdg, ngraham
Cc: filipf, ngraham, nicolasfella, zzag, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200320/82ee6b55/attachment-0001.html>