Bug 32267 - Implement handling Client.Observer.DelayApprovers
Summary: Implement handling Client.Observer.DelayApprovers
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Jonny Lamb
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/jo...
Whiteboard:
Keywords: patch
Depends on:
Blocks: 34964
  Show dependency treegraph
 
Reported: 2010-12-09 08:31 UTC by Jonny Lamb
Modified: 2011-03-03 04:19 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonny Lamb 2010-12-09 08:31:40 UTC
http://git.collabora.co.uk/?p=user/jonny/telepathy-spec.git;a=shortlog;h=refs/heads/observer

That's the spec branch. Implement it.
Comment 1 Jonny Lamb 2010-12-21 08:02:35 UTC
howzat?
Comment 2 Simon McVittie 2011-01-03 04:52:59 UTC
I can't find a bug# regarding the spec, but it looks good.

Implementation looks good, but I'd also like to see a test in which the Observer follows the pattern from Bug #27860 (with either Claim or HandleWith), and as a result, ADO isn't called *at all*; for the cases mentioned in that bug (and in the spec branch here), it seems as though that's the important thing we want.

Relatedly, it would be really great (but isn't a merge blocker for this) if you could fix Bug #27860 (document that observers can act like non-interactive approvers). Perhaps the situations Will mentions there could help to make this bug's spec rationale more concrete, too.
Comment 3 Jonny Lamb 2011-01-03 07:30:52 UTC
Okay cool, I added a test to do what bug #27860 describes, and I guess it's just as well you asked for it as I had to fix MC to not assert.

Branch updated.
Comment 4 Simon McVittie 2011-01-03 09:14:31 UTC
+        if (approver_event_id > 0)
+        {
+            DEBUG ("Cancelling call to approvers as dispatch operation has been Claimed");
+            g_source_remove (approver_event_id);
+        }

This only seems to happen for Claim: what happens if a non-interactive approver calls HandleWith?
Comment 5 Jonny Lamb 2011-01-04 03:28:45 UTC
(In reply to comment #4)
> This only seems to happen for Claim: what happens if a non-interactive approver
> calls HandleWith?

I've added a test. It did accidentally work, but I've made it work a little more obviously. See my branch!
Comment 6 Guillaume Desmottes 2011-03-01 02:10:01 UTC
Spec: I think it would be good to document what's the "default" value of this property: i.e. how is MC supposed to deal with an old Observer not implementing the property.
Comment 7 Guillaume Desmottes 2011-03-01 04:05:25 UTC
(In reply to comment #6)
> Spec: I think it would be good to document what's the "default" value of this
> property: i.e. how is MC supposed to deal with an old Observer not implementing
> the property.

I've done this in http://git.collabora.co.uk/?p=user/cassidy/telepathy-spec;a=shortlog;h=refs/heads/delay-approver

Jonny ++ed it, so we can merge it at the same time as the implementation.
Comment 8 Guillaume Desmottes 2011-03-03 02:34:36 UTC
For the record, I opened bug #34964 about the implementation of DelayApprovers in TpBaseClient.
Comment 9 Guillaume Desmottes 2011-03-03 04:19:17 UTC
I merged the spec branch (will be in 0.21.11) and the MC one (will be in 5.7.6).


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.