apol added a comment.
Looking much much better, please see new comments. INLINE COMMENTS > FindSoup.cmake:1 > +# FindSoup.cmake > +# <https://github.com/nemequ/gnome-cmake> Are you sure this file is till necessary? Where is libsoup used? > CMakeLists.txt:47 > +endif() > \ No newline at end of file Please add > FwupdBackend.cpp:85 > +{ > + const QString name = QLatin1String(fwupd_device_get_name(device)); > + FwupdResource* res = new FwupdResource(name, true, this); When getting a string from glib, use QString::fromUtf8, which is what I guess they use. > FwupdBackend.cpp:328 > + GPtrArray *checksums; > + FwupdResource* app = NULL; > + Don't set to null only to initialize at the next line. > FwupdBackend.cpp:396 > + QFile file(filename_cache.toString()); > + app->m_file = &file; > + // To Do Download for a file? &file is in this scope, it will crash as soon as you call m_file outside this scope. Maybe just keep the path? > FwupdBackend.cpp:411 > + file.write(Data); > + } > + file.close(); else { qWarning() ... > FwupdBackend.cpp:657 > +{ > + return NULL; > +} Use nullptr everywhere you use NULL. > FwupdBackend.h:48 > +} > +#include <gio/gio.h> > + gio doesn't seem necessary anymore? > FwupdResource.h:114 > + > + QList<FwupdResource*> m_releases; // A list of all refrences to releases > of a device. > +}; Use QVector > FwupdSourcesBackend.cpp:152 > +{ > + qWarning() << "Removal of Sources Not Allowed" << "Remote-ID" << id; > + return false; How come it's possible to add but not to remove? Is it just a TODO? > FwupdSourcesBackend.h:43 > + bool removeSource(const QString& id) override; > + QString idDescription() override { return QStringLiteral(""); } > + QList<QAction*> actions() const override; Maybe pass the repostory url as description? Also to return qn empty string use either {} or QString(). > FwupdTransaction.cpp:26 > +#include <QDebug> > +#include <KRandom> > + ? > FwupdTest.cpp:148 > + const auto resources = fetchResources(m_appBackend->search({})); > + QCOMPARE(m_appBackend->property("startElements").toInt()*2, > resources.count()); > + This is just copied from the dummy test, it doesn't make sense here... REPOSITORY R134 Discover Software Store REVISION DETAIL https://phabricator.kde.org/D14050 To: abhijeet2096, apol, davidedmundson Cc: zzag, anthonyfieroni, plasma-devel, ragreen, ixoos, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
