ngraham added inline comments.

INLINE COMMENTS

> advanceconfig.cpp:207
> +        qDebug() << job->errorText();
> +        KMessageBox::error(this, i18n("Synchronization failed."));
> +    } else {

"Synchronization failed." is a pretty frustrating error message. The user will 
wonder, "How did it fail? What happened? How can I fix it?" etc. Since we have 
the error text, let's show it in the message box, since it could provide some 
clues.

> advanceconfig.cpp:211
> +        qDebug() << "Synchronization successful";
> +        KMessageBox::information(this, i18n("Synchronization successful."));
> +    }

I don't think we need a dialog box for the success case. That'll just annoy 
people.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  sddm-theme-syncing (branched from master)

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

To: filipf, #plasma, ngraham, davidedmundson, #vdg
Cc: leinir, cfeck, GB_2, ndavis, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

Reply via email to