[Differential] [Requested Changes To] D2345: use a separate configuration per look and feel

[davidedmundson (David Edmundson)]

davidedmundson requested changes to this revision.
davidedmundson added a reviewer: davidedmundson.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> shellcorona.cpp:469
> +
> +void ShellCorona::updateLookAndFeelPackage(const QString &file)
> +{

why are we tracking file modifications?
That's not how the rest of the KCMs work.

> shellcorona.cpp:511
> +    //the old config file name
> +    //TODO:alternative: kconfigupdate?
> +    if (m_lookAndFeelPackage.metadata().pluginId() != "org.kde.breeze.desktop") {

yes, otherwise everyone currently using breeze-dark loses their config on upgrade.

> shellcorona.cpp:720
> +        //this form doesn't crash, while qDeleteAll(containments()) does
> +        delete containments().first()->property("_plasma_graphicObject").value<QObject *>();
> +        delete containments().first();

That's clearly not how Plasma should be designed.

REPOSITORY
  rPLASMAWORKSPACE Plasma Workspace

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma, davidedmundson
Cc: davidedmundson, ivan, plasma-devel, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160803/b80153a1/attachment.html>