Bug 30000 - Implement final version of SucceededWithChannel, CreateChannelWithHints etc.
Summary: Implement final version of SucceededWithChannel, CreateChannelWithHints etc.
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: 5.7
Hardware: All All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/ca...
Whiteboard:
Keywords: patch
Depends on: request-hints 30092
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-03 09:04 UTC by Simon McVittie
Modified: 2010-12-22 02:57 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2010-09-03 09:04:39 UTC
+++ This bug was initially created as a clone of Bug #28866 +++

See that bug for initial discussion.
Comment 1 Simon McVittie 2010-09-03 09:06:56 UTC
Next steps here are (quoting cassidy and myself from <https://bugs.freedesktop.org/show_bug.cgi?id=28866#c10>):

> I fixed this one (not sure if that's the best way, this code looks like a bit
> like a can of worms to me tbh :\ )

This code *is* a can of worms :-( See smcv/wip-req-export, in which I tried to
split McdChannel into the bits that are really a channel and the bits that are
a channel request, and have the latter be exported as the ChannelRequest; in
particular, the top commit, which is called "WiP: doesn't work" and is
something like my 5th attempt at it.

Your code changes look good, except for the last commit where I need to read
the code a bit more to work out wtf is going on. I suspect that what you did is
necessary and correct, even though it makes little sense.

Would you mind if I pick up this branch and make it use an in-tree extension
instead of the patched telepathy-glib you're presumably using, so we can merge
it before undrafting the spec, and continue to refactor?
Comment 2 Simon McVittie 2010-09-06 09:29:55 UTC
The referenced branch takes all the commits I didn't object to from Guillaume's branch, and replaces the last commit (which we were both suspicious about) with a version that I think makes more sense.
Comment 3 Guillaume Desmottes 2010-09-07 01:25:41 UTC
Looks good (assuming all the tests pass now).

I'm happy to see that the general desgin was actually as crack as I thought. :)
Your comment makes things a bit clearer but it would be good to properly refactor all this mess at some point.
Comment 4 Simon McVittie 2010-09-07 04:57:24 UTC
(In reply to comment #3)
> Looks good (assuming all the tests pass now).

They do. Fixed in git for 5.5.4; I've left this bug open for implementation of the final spec for this.

> I'm happy to see that the general desgin was actually as crack as I thought. :)
> Your comment makes things a bit clearer but it would be good to properly
> refactor all this mess at some point.

Yeah, I know; I introduced McdRequest as a way to work towards that, and will continue in that direction.
Comment 5 Simon McVittie 2010-09-09 06:52:56 UTC
Bug #30092 continues the refactoring.

The branch referenced here, which follows on from the one on Bug #30092, adds request-properties to the Handler_Info as well as the Observer_Info, which is mandated by current spec git (the forthcoming 0.19.12); this will probably be good to have before undrafting this branch.
Comment 6 Guillaume Desmottes 2010-10-11 07:26:13 UTC
+ * @paths_out: (transfer container) (element-type utf8): Requests_Satisfied
+ * @props_out: (transfer container) (element-type utf8 GHashTable):
+ *  request-properties for Observer_Info or Handler_Info

Shouldn't you add (out) as well? (We don't really care here but if we use gir annotation, best to get them right).

Except that, branch looks good.
Comment 7 Simon McVittie 2010-10-11 10:51:40 UTC
Branch merged to master with the obvious change; it'll be in 5.7.0. Leaving this bug open for the final version of the spec, when that lands.
Comment 8 Simon McVittie 2010-11-17 07:07:08 UTC
My 'hints' branch updates MC to my spec proposal from Bug #28866 (read that bug first).
Comment 9 Guillaume Desmottes 2010-11-17 07:23:13 UTC
++
Comment 10 Simon McVittie 2010-11-17 08:01:45 UTC
Thanks, merged.
Comment 12 Guillaume Desmottes 2010-12-22 02:57:47 UTC
Merged to master for 5.7.1


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.