D28154: Add users KCM

[David Edmundson]

davidedmundson added a comment.


  Which release?
  
  We need to remove user-manager. We're technically in a repo freeze, and this would involve changing (removing) a repo a breakage of that.
  
  --------
  
  Generally this looks quite neat and tidy. A big simplification +++

INLINE COMMENTS

> kcm.cpp:94
> +    KQuickAddons::ConfigModule::save();
> +    Q_EMIT apply();
> +}

what's this for?

> kcm.cpp:97
> +
> +QString KCMUser::initializeString(const QString& stringToInitial)
> +{

This is an odd method name.

I don't really understand what it's doing, some sort of title casing?

> kcm.cpp:121
> +    QTemporaryFile file;
> +    file.setAutoRemove(false);
> +    if (file.open()) {

so who cleans this up?

> user.cpp:96
> +    }
> +    QString trueVal = value.toString();
> +    if (value.toString().startsWith(QStringLiteral("file://"))) {

use url.path and url.protocol for this not string manipulation

> usermodel.cpp:72
> +        connect(user, &User::dataChanged, [=]() {
> +            beginResetModel();
> +            endResetModel();

why are we resetting the whole model instead of just dataChanged() on the relevant row

> usermodel.cpp:78
> +    std::sort(m_userList.begin(), m_userList.end(), [](User *lhs, User *) {
> +        return lhs->loggedIn();
> +    });

The ones not logged in will be sorted randomly.

Also note that if the intention is to have your user at the top, this check won't suffice as you can have 2 things logged in.

> broulik wrote in usermodel.cpp:96
> You're leaking all the users.
> `qDeleteAll(m_userList)`

this is marked as done, yet I can't see where ?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  arcpatch-D28154

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

To: cblack, #plasma, #vdg, ngraham
Cc: mart, yurchor, iasensio, meven, crossi, The-Feren-OS-Dev, davidedmundson, broulik, filipf, ngraham, nicolasfella, zzag, plasma-devel, Orage, LeGast00n, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200507/c4f95234/attachment-0001.htm>