apol added a comment.
Some coding style suggestions.
General +1.
INLINE COMMENTS
> knotificationmanager.cpp:85
> + const QString runtimeDir = qgetenv("XDG_RUNTIME_DIR");
> + if (!runtimeDir.isEmpty()) {
> + d->inSandbox = QFile::exists(runtimeDir +
> QLatin1String("/flatpak-info"));
Use `qEnvironmentVariableIsEmpty`?
> notifybyflatpak.cpp:2
> +/*
> + Copyright (C) 2005-2006 by Olivier Goffart <ogoffart at kde.org>
> + Copyright (C) 2008 by Dmitry Suzdalev <dim...@gmail.com>
Are you sure so many people? :P
> notifybyflatpak.cpp:112
> + watcher->addWatchedService(portalDbusServiceName);
> + connect(watcher, SIGNAL(serviceOwnerChanged(QString,QString,QString)),
> + SLOT(onServiceOwnerChanged(QString,QString,QString)));
Use new connect syntax.
> notifybyflatpak.cpp:255
> + for (int i = 0; i < actionList.count(); i += 2) {
> + QVariantMap button;
> + button.insert(QStringLiteral("action"), actionList.at(i));
Something like
QVariantMap button = {
{ StringLiteral("action"), actionList.at(i)},
{QStringLiteral("label"), actionList.at(i + 1)}
};
would be more readable?
> notifybyflatpak.cpp:297
> + args.append(QString::number(id));
> + m.setArguments(args);
> +
`m.setArguments({ QString::number(id) });`
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D4425
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: jgrulich, mck182
Cc: apol, #frameworks
