Add support for applications in KNSBackend
ClosedPublic

Authored by leinir on Mar 17 2019, 5:11 PM.

Details

Summary

Prior to this patch, when loading a knsrc file which would show applications from the KDE Store, the entries would be shown in the Application Addons menu. This patch adds functionality to put those into the Application category, and further adds filtering on architecture for those packages (so we only show entries with downloadable content supported by the machine we are running on).

Diff Detail

Repository
R134 Discover Software Store
Lint
Lint Skipped
Unit
Unit Tests Skipped
leinir created this revision.Mar 17 2019, 5:11 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 17 2019, 5:11 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
leinir requested review of this revision.Mar 17 2019, 5:11 PM

Can you edit the test plan to indicate the name of an app from the store that now appears in Discover's Applications section? A quick test of this patch didn't reveal ExeQt or Treeline in Discover, which are visible on http://opendesktop.org.

libdiscover/backends/KNSBackend/KNSBackend.cpp
125

Is this intentional? Don't we want to initialize it right after creating it?

Can you edit the test plan to indicate the name of an app from the store that now appears in Discover's Applications section? A quick test of this patch didn't reveal ExeQt or Treeline in Discover, which are visible on http://opendesktop.org.

Applications will show up if a knsrc which provides them (specifically storekdeapps.knsrc) is installed... I did in fact ponder whether it would make sense for us to ship said knsrc file with Discover, but also am unsure if that is the right place for it...

libdiscover/backends/KNSBackend/KNSBackend.cpp
125

It is intentional, and ensures that just on the wild offchance we get any error signals during that initialisation are caught. It would be sufficient to put this after the signalErrorCode connect statement, but it just seems reasonable and pleasant to have those connect statements together :)

apol added inline comments.Mar 18 2019, 10:06 AM
libdiscover/backends/KNSBackend/KNSBackend.cpp
151

we can just compare with == here, it will be easier to read.

159

I wonder if that's alright, it's very ad-hoc to the opendesktop semantics, isn't it?

186

decoration isn't used anymore, it could make sense to not include it at all over here.

242

Unrelated change.

469

Just use contains?

leinir marked 4 inline comments as done.Mar 18 2019, 11:42 AM
leinir added inline comments.
libdiscover/backends/KNSBackend/KNSBackend.cpp
151

Tru dat... i was just keeping consistency with the long list below, but yeah, at the very least for now we've just got the one source, and can always add a check like this if we find we have more app sources later on :)

159

This is pretty much just mapping the current machine's abilities to run certain types of binaries to what is offered by opendesktop, yes. I am quite unsure as to how to implement this in some other way. In particular, we need to be able to make sure that on x86_64 we get both x86 and x86_64 packages, so we can't just send along the result verbatim and ask opendesktop to return what Qt returns (given we have to produce a mapping anyway)...

186

Good idea :) This is just code moved down a bit, but i'll just pop that in here as well while we're at it :)

leinir updated this revision to Diff 54204.Mar 18 2019, 11:43 AM
leinir marked 2 inline comments as done.

Address various comments, and also fix one issue discovered while addressing those

apol accepted this revision.Mar 18 2019, 1:15 PM
This revision is now accepted and ready to land.Mar 18 2019, 1:15 PM
This revision was automatically updated to reflect the committed changes.