| Summary: | [next] replace TpBase*Class::interfaces by TpBase*Class::get_interfaces | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Xavier Claessens <xclaesse> |
| Component: | tp-glib | Assignee: | Telepathy bugs list <telepathy-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | Keywords: | patch |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://cgit.freedesktop.org/~jonny/telepathy-glib/log/?h=51441-get-interfaces | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Bug Depends on: | |||
| Bug Blocks: | 31668 | ||
|
Description
Xavier Claessens
2012-06-26 04:12:58 UTC
Okay I worked on this in master. Once merged, I'll remove all the old stuff in next. (In reply to comment #0) > TpBaseChannel, TpBaseCallContent and TpBaseCallStream use that system, but not > TpBaseConnection, Done. > TpBaseConnectionManager, Done. > TpBaseRoomConfig and This doesn't actually need this? I did fix a comment referring to TpBaseChannel.interfaces though. > TpBaseProtocol. This already has a get_interfaces() which returns a newly-allocated GStrv (why, I wonder?) This makes is harder to chain-up. Do you think another get_interfaces_array (which uses a GPtrArray) should be added for master, then renamed in next? (In reply to comment #1) > Okay I worked on this in master. Once merged, I'll remove all the old stuff in > next. Perfect! > (In reply to comment #0) > > TpBaseRoomConfig and > > This doesn't actually need this? I did fix a comment referring to > TpBaseChannel.interfaces though. True > > TpBaseProtocol. > > This already has a get_interfaces() which returns a newly-allocated GStrv (why, > I wonder?) This makes is harder to chain-up. Do you think another > get_interfaces_array (which uses a GPtrArray) should be added for master, then > renamed in next? Yeah, this one is sad... I would vote for doing get_interfaces_array() in master and rename it in next. I would also use _TP_SEAL(interfaces) to make them easily catchable at build time, but that means they need to be out of the doc, see http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=e8f9a27a388e2123a3281d610acbcd6a4b17d9a1 tp_base_connection_add_interfaces(): doing a g_strv_length() to set the size of the array is totally overkill IMO. It means you iterate the array twice with probably no benefit since GArray already pre-allocate more than needed. We are not dealing with hundred of ifaces here, GArray's minimum allocation is already 8 elements, we probably will never have more ifaces than that anyway. (In reply to comment #2) > Yeah, this one is sad... I would vote for doing get_interfaces_array() in > master and rename it in next. Alright, done. > I would also use _TP_SEAL(interfaces) to make them easily catchable at build > time, but that means they need to be out of the doc, see > http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=e8f9a27a388e2123a3281d610acbcd6a4b17d9a1 Done. > tp_base_connection_add_interfaces(): > doing a g_strv_length() to set the size of the array is totally overkill IMO. > It means you iterate the array twice with probably no benefit since GArray > already pre-allocate more than needed. We are not dealing with hundred of > ifaces here, GArray's minimum allocation is already 8 elements, we probably > will never have more ifaces than that anyway. I' simplified this function. +1 (In reply to comment #4) > +1 Cool, thanks. I've merged this to master. I'll remove all the deprecated stuff when merging into next. |
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.