apol requested changes to this revision. apol added inline comments. This revision now requires changes to proceed.
INLINE COMMENTS > SourcesPage.qml:113 > } > + > onObjectAdded: { Remove unrelated patch > CMakeLists.txt:47 > +endif() > \ No newline at end of file See warning > CMakeLists.txt:29 > +add_library(fwupd-backend MODULE ${fwupd-backend_SRCS}) > +target_link_libraries(fwupd-backend Qt5::Core Qt5::Widgets KF5::CoreAddons > KF5::ConfigCore Discover::Common LIBFWUPD ${Soup} ${GIO} ${GLib} ) > + Soup, GIO and GLib are dependencies in LIBFWUPD, no? if so do the search and link in FindFWUPD.cmake. > FwupdBackend.cpp:100 > + { > + AbstractResource* res = (AbstractResource*) r; > + if(r->m_id.isEmpty()) No need to cast. Use the FwupdResource directly. > FwupdBackend.cpp:167 > + if (fwupd_device_has_flag (dev, FWUPD_DEVICE_FLAG_UPDATABLE)) > + res->isLiveUpdatable = true; > + if (fwupd_device_has_flag (dev, FWUPD_DEVICE_FLAG_ONLY_OFFLINE)) ` res->isLiveUpdatable = fwupd_device_has_flag (dev, FWUPD_DEVICE_FLAG_UPDATABLE)` And same with the rest. > FwupdBackend.cpp:193 > + { > + vendor_name = g_strdup_printf ("%s %s",fwupd_device_get_vendor > (dev), fwupd_device_get_name (dev)); > + } Use QString, not g_* to construct strings. > FwupdBackend.cpp:209 > +{ > + g_autoptr(GPtrArray) devices = NULL; > + Just initialize at once. No need to set NULL first. > FwupdBackend.cpp:219 > + FwupdDevice *device = (FwupdDevice *)g_ptr_array_index (devices, > i); > + FwupdResource * res = NULL; > + g_autoptr(GPtrArray) releases = NULL; Same > FwupdBackend.cpp:233 > + > + if (releases != NULL) > + { `if (releases)` > FwupdBackend.cpp:235 > + { > + for (int j = 0; j < (int)releases->len; j++) > + { Use uint rather than casting > FwupdBackend.cpp:261 > + /* get All devices, Will filter latter */ > + devices = fwupd_client_get_devices (client, cancellable, &error); > + if(devices == NULL){ Don't initialize twice. > FwupdBackend.cpp:262 > + devices = fwupd_client_get_devices (client, cancellable, &error); > + if(devices == NULL){ > + if (g_error_matches (error,FWUPD_ERROR,FWUPD_ERROR_NOTHING_TO_DO)){ if (devices) > FwupdBackend.cpp:269 > + } > + } > + else{ } else { > FwupdBackend.cpp:464 > + > + if (!g_file_set_contents > (filename,msg->response_body->data,msg->response_body->length,&error_local)) > + { Use QFile > FwupdBackend.cpp:482 > + /* local */ > + if (g_str_has_prefix (uri, "file://")) { > + gsize length = 0; QUrl > FwupdBackend.h:35 > +#include <gio-unix-2.0/gio/gunixfdlist.h> > +#include <glib/gi18n.h> > +#include <glib/gstdio.h> We use the i18n coming from KLocalizedString > FwupdNotifier.cpp:37 > +{ > + emit foundUpdates(); > +} Don't add one that does nothing. Either implement it or just don't add it. > FwupdResource.cpp:153 > +{ > + m_state = state; > + emit stateChanged(); Check that it's different before setting/emitting. > FwupdReviewsBackend.cpp:30 > + > +FwupdReviewsBackend::FwupdReviewsBackend(FwupdBackend* parent) > + : AbstractReviewsBackend(parent) This is all copied from the dummy backend and doesn't apply. Remove. > FwupdUpdater.cpp:76 > + > +quint64 FwupdUpdater::downloadSpeed() const > +{ Would it make sense to use the StandardUpdater? > fwupd-backend-categories.xml:9 > + <Include> > + <And> > + <Category>Firmware Updates</Category> I wouldn't include the category file for now. 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