Differential D27954

appstreamrunner: Port to KApplicationTrader
ClosedPublic
Actions

Authored by apol on Mar 9 2020, 6:01 PM.

Details

Reviewers
dfaure
Group Reviewers
Plasma
Commits
R120:3f0ca0da37d8: appstreamrunner: Port to KApplicationTrader
Summary

Port away from the query language to lambda using the new KApplicationTrader API.

Depends on D27953.

Test Plan

Tested by hand with several applications, still works as expected.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.Mar 9 2020, 6:01 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 9 2020, 6:01 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.Mar 9 2020, 6:01 PM
dfaure requested changes to this revision.Mar 11 2020, 10:15 PM

Cool to see KApplicationTrader being used :-)

runners/appstream/appstreamrunner.cpp
96

Note that the original trader query used =~ which meant "case insensitive comparison".
I have no idea if it was necessary though.

102

This test seems slow and unnecessary.
IMHO you can remove it, and just query for that property, and if it's empty it won't contain anything so you'll move ahead to the final return false.

This revision now requires changes to proceed.Mar 11 2020, 10:15 PM
apol updated this revision to Diff 77469.Mar 11 2020, 11:06 PM

Remove unnecessary check

apol added inline comments.Mar 11 2020, 11:07 PM
runners/appstream/appstreamrunner.cpp
96

Might have just chosen the wrong operator, it's part of the problem of using a weird ad-hoc language I guess.

dfaure accepted this revision.Mar 12 2020, 1:30 AM
dfaure added inline comments.
runners/appstream/appstreamrunner.cpp
102

For symmetry I would have removed the "else" altogether, there are many other cases of if()+return above already, no need to do things differently here.

This revision is now accepted and ready to land.Mar 12 2020, 1:30 AM
Closed by commit R120:3f0ca0da37d8: appstreamrunner: Port to KApplicationTrader (authored by apol). · Explain WhyMar 14 2020, 12:51 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 77602

View Options

runners/appstream/appstreamrunner.cpp

Loading...