broulik accepted this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  Thanks
  
  Can this go into 5.18?

INLINE COMMENTS

> main.cpp:147
> +        const QModelIndex index = m_iconSizeCategoryModel->index(i, 0);
> +        if (!index.isValid()) {
> +            continue;

I think this check isn't necessary

> main.cpp:150
> +        }
> +        const QString key = m_iconSizeCategoryModel->data(index, 
> IconSizeCategoryModel::ConfigKeyRole).toString();
> +        if (m_settings->findItem(key)->isSaveNeeded()) {

You can also do `index.data(role)`

> main.cpp:307
>          for (int i = 0; i < KIconLoader::LastGroup; i++) {
> -            KIconLoader::emitChange(KIconLoader::Group(i));
> -
>              QDBusMessage message = 
> QDBusMessage::createSignal(QStringLiteral("/KGlobalSettings"),
>                                                                
> QStringLiteral("org.kde.KGlobalSettings"),

Isn't this redundant? i.e. doesn't `emitChange` emit do exactly this?

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, bport, broulik
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Reply via email to