ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kcm.cpp:277
> + if (!m_currentBehavior) {
> + setBehaviorSettingsToLoad(m_behaviorSettingsList.begin().key());
> + }
What about m_behaviorSettingsList is empty? I'm not sure we can guarantee that
it'll always contain something.
> kcm.cpp:321
> + bool needSave = false;
> + for (auto *behaviorSettings : m_behaviorSettingsList) {
> + needSave |= behaviorSettings->isSaveNeeded();
Replace with any_of ? This would break the loop earlier.
> kcm.h:93
>
> + Q_INVOKABLE void setBehaviorSettingsToLoad(QString identifier);
> +
Should be a const ref
> kcm.h:108
> + bool isSaveNeeded() const override;
> + void createConnections(NotificationManager::BehaviorSettings* settings);
>
Space before * not after
> sourcesmodel.cpp:380
> + QVector<QPair<QString, QString>> res;
> + for (auto &source : qAsConst(m_data)) {
> + if (source.desktopEntry.isEmpty()) {
const auto &
> sourcesmodel.h:98
> + // List of config settings in the form of Application, DesktopEntry of
> Services, NotifyrcName to load.
> + QVector<QPair<QString, QString>> settingsList() const;
> +
It's generally a better idea to define a small trivial struct when roll with
QPair
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D27188
To: crossi, #plasma, ervin, broulik, bport, meven
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
