D29175: DBus Runner: Add service property to request actions once

2020-05-07 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> alex wrote in dbusrunnertest.cpp:206
> The concept behind this test is, that the runner is just loaded. If the 
> Request-Actions-Once property is correctly implemented the actions are 
> requested when the plugin is initialized. That means that they should be 
> available and `prepare` doesn't need to be called.
> 
> So it would make sense to add a signal spy to check if prepare was not called 
> at all, or am I missing something?

I am not sure the test illustrates the change.
1 action available means it did not regress but you don't test the actual new 
behavior AFAICT

Only by adding signals spies can you test the actual behavior here.

I would trigger a second match and check requestActions was only called once, 
but prepare called each time as usual.

REPOSITORY
  R308 KRunner

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-05-02 Thread Alexander Lohnau 
alex added inline comments.

INLINE COMMENTS

> meven wrote in dbusrunnertest.cpp:206
> You should have added a QSignalSpy to check prepare was called, and another 
> for requestAction to check it has been called only once.

The concept behind this test is, that the runner is just loaded. If the 
Request-Actions-Once property is correctly implemented the actions are 
requested when the plugin is initialized. That means that they should be 
available and `prepare` doesn't need to be called.

So it would make sense to add a signal spy to check if prepare was not called 
at all, or am I missing something?

REPOSITORY
  R308 KRunner

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-05-01 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> dbusrunnertest.cpp:204
> +// We haven't called the prepare slot, if the implementation works
> +// the actions should alredy be available
> +auto actions = m.actionsForMatch(fakeMatch);

typo: already

> dbusrunnertest.cpp:206
> +auto actions = m.actionsForMatch(fakeMatch);
> +QCOMPARE(actions.count(), 1);
> +

You should have added a QSignalSpy to check prepare was called, and another for 
requestAction to check it has been called only once.

REPOSITORY
  R308 KRunner

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-05-01 Thread Alexander Lohnau 
This revision was automatically updated to reflect the committed changes.
Closed by commit R308:7b6222d8a10b: DBus Runner: Add service property to 
request actions once (authored by alex).

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29175?vs=81623=81687

REVISION DETAIL
  https://phabricator.kde.org/D29175

AFFECTED FILES
  autotests/dbusrunnertest.cpp
  autotests/dbusrunnertest.desktop
  src/data/servicetypes/plasma-runner.desktop
  src/dbusrunner.cpp

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-05-01 Thread Nathaniel Graham 
ngraham accepted this revision.

REPOSITORY
  R308 KRunner

BRANCH
  request_actions_once (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-05-01 Thread Alexander Lohnau 
alex added a dependent revision: D29320: Baloo Runner: Use 
X-Plasma-Request-Actions-Once property in service.

REPOSITORY
  R308 KRunner

BRANCH
  request_actions_once (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-04-30 Thread Alexander Lohnau 
alex updated this revision to Diff 81623.
alex added a comment.


  Create test

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29175?vs=81165=81623

BRANCH
  request_actions_once (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29175

AFFECTED FILES
  autotests/dbusrunnertest.cpp
  autotests/dbusrunnertest.desktop
  src/data/servicetypes/plasma-runner.desktop
  src/dbusrunner.cpp

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-04-30 Thread Méven Car 
meven accepted this revision.
meven added a comment.
This revision is now accepted and ready to land.


  Seems good to me.
  Before merging, maybe you could make a diff with a few runners in 
plasma-workspace use it.
  And if you could add a test.

REPOSITORY
  R308 KRunner

BRANCH
  request_actions_once (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-04-26 Thread Alexander Lohnau 
alex added a comment.


  No, they are independent.
  I just mentioned this to avoid confusion when debugging.

REPOSITORY
  R308 KRunner

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-04-26 Thread Méven Car 
meven added a comment.


  Is it a dependency or alternative to D29050 
 ?

REPOSITORY
  R308 KRunner

REVISION DETAIL
  https://phabricator.kde.org/D29175

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

2020-04-25 Thread Alexander Lohnau 
alex created this revision.
alex added reviewers: Plasma, meven, ngraham, broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
alex requested review of this revision.

REVISION SUMMARY
  With the `X-Plasma-Request-Actions-Once` property the plugin
  can specify if the actions should only be requested when the plugin is 
initialized.
  
  This is useful if the plugin always used the same actions, because
  then the we don't need to make the dbus calls for every match session.

TEST PLAN
  Create dbus runner and don't set the extra property, the actions should be 
requested for each session.
  (More like each query because of BUG: 420311).
  After setting the property to true the actions should only be requested when 
the runner is initialized.

REPOSITORY
  R308 KRunner

BRANCH
  request_actions_once (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29175

AFFECTED FILES
  src/data/servicetypes/plasma-runner.desktop
  src/dbusrunner.cpp

To: alex, #plasma, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns