Merge lp://staging/~sylvain-pineau/checkbox/fix-1227684 into lp://staging/checkbox

tentative -1 before I talk to you about implications, also this makes the API even uglier :-/

I'm open to a better API for this of course but so far the signal only set the job outcome to OUTCOME_UNDECIDED in such cases. The -temporary- job result that we get from running the job command with the Test button is not transmitted at all. I'm not for creating a new OUTCOME_UNDECIDED_PASS/FAIL that's why passing a second parameter seemed to be a good idea.

First of all, let's fix the argument name, call it 'suggestion', maybe or make a better name.

Then passing suggestions is *okay* but why are we passing SKIP as a suggestion? I know the answer, because the GUI is limited and that's how it works (it needs that signal so we send it even though it makes no sense).

Can we please try to keep the silliness out of the backend code and keep it in the UI, and remove it as we go?

How about you add a signal ShowInteractiveUI() instead of that AskForOutcome(self, SKIP) and treat it just the same for now. This will, at least, make the dbus API explicit and saner and hide reliance on quirky UI behavior. Then the AskForOuntcome() signal can send the suggewstion as you do now.

I'm a bit worried that it's too generic though, only commands where the return value means something should send the suggestion. We should explicitly enable that, otherwise we'll get trigger-happy "PASS" for commands that echo "this has failed" with exit code 0 from echo.

Ping?

OK, now I'm using two different signals in order to avoid creating an horrible API.

17:49 <@zyga> spineau: I think you can omit the argument to ShowInteractiveUI if the sender is always that job
17:49 <@zyga> spineau: alternatively move the signal to the session class and retain the argument, it would make more sense there
17:49 <@zyga> (no need to track changing objects)
17:50 <@zyga> spineau: otherwise looks okay
17:50 <@zyga> spineau: one thing I don't remember and I cannot see in the diff alone is if that's making the backend logic simplier
17:50 <@zyga> spineau: are thare many / any remaining calls to AskForOutcome()?

17:51 <@spineau> zyga: AskForOutcome is only used in _result_ready now
17:52 <@zyga> spineau: ok, please move the signal to session and let's land this
17:52 <@zyga> spineau: if you can (want to) move the other signal as well, it would be consistent this way and probably, again, easier
17:53 <@spineau> zyga: humm, about moving the signal to the session, it means that the session has a way to know the "active" primed job I'm not sure if it's possible
17:53 <@zyga> spineau: no, it only means that the place that calls that would need to know which session it came from, that's true anyway
17:53 <@zyga> spineau: the signal is on the session, the same code would trigger it, right?
17:54 <@spineau> zyga: ok, signals are in the session but the primed job only has a pointer to the session_wrapper
17:54 <@spineau> zyga: so I just need to find the native object of this path
17:55 -!- knownew [~knownew@193.28.144.135] has joined #checkbox
17:57 <@spineau> zyga: forget it, I misread the attribute
17:57 <@spineau> name
18:01 <@zyga> spineau: hmm, well, that's all that you nead
18:01 <@zyga> spineau: do you think that is sensible?
18:02 <@spineau> zyga: no, it should be ok, testing the move now
18:03 <@zyga> spineau: ok

ok both signals have been moved to the session wrapper.

Please correct commit message to indicate this move had occured and self-approve, thanks.

self-approved with an updated commit msg.