When we break D-Bus API we should also break C API: - remove tp_get_bus (Bug #18832) - don't emit TpProxy::invalidated in dispose (Bug #22119) - remove all _run_ methods (Bug #22207) - don't allow get_contact_statuses to fail (Bug #21796) - get rid of everything deprecated - replace TpChannelManagerIface.foreach_channel_class with a renamed TpChannelManagerIface.type_foreach_channel_class - replace the two original TpClientChannelFactory methods with a renamed obj_* - return void from tp_proxy_add_interface_by_id (Bug #21795) Other possible things: - get TpContact objects in a more GIO-ful way (Bug #27687), although this might be superseded by fixing handles - make all async functions not re-enter (using an idle if necessary) - check everything for introspectability and fix troublesome APIs as much as possible ------------------------- Possibly to do, or not, depending how long the above take: - cut as much as possible of the dbus-glib out of our API/ABI - move to GDBus, which is probably also an ABI break, but might have to be delayed if the above take too long (Bug #28782)
Additional minor things from FIXME.out: * change tp_error_quark()'s quark to tp-error-quark * change a GValue* in the middle of a struct to a GValue in properties-mixin.h (or just delete the entire properties mixin)
Also move ChatStates support from TpChannel to TpTextChannel, following tp-qt4's lead.
13:04 < cassidy> tomeu, smcv: do you know about this kind of warnings : _gen/telepathy-enums.h:26: Warning: TelepathyGLib: symbol='NUM_TP_HANDLE_TYPES': Unknown namespace for symbol 'NUM_TP_HANDLE_TYPES' 13:05 < smcv> cassidy: it wanted us to use TP_NUM_HANDLE_TYPES or some such 13:05 < smcv> cassidy: tbh just ignore it, and make sure we use the latter in the 1.0 codegen
*** Bug 17646 has been marked as a duplicate of this bug. ***
We now have a 'next' branch where we're doing this. \o/ Some things that need changing in 'next': * --generate-all-reentrants should be --separate-reentrants or something * Version should be < 1.0 at first (0.99.n where n is a snapshot number incremented when it's halfway stable for a bit? 0.2012.n because this is the end of the world? ...) and become 1.0 when the core library reaches API/ABI stability * Commented-out Call stuff would be better #ifdef'd? * "Presence Presence interface" * tp_message_to_text should still return whether there's non-text content
Created attachment 56410 [details] [review] Excise Tubes from channel.xml to fix build
Created attachment 56411 [details] [review] Remove deprecated, unused debug-ansi.h
Created attachment 56412 [details] [review] Remove deprecated tp_message_new()
(In reply to comment #6) > Created attachment 56410 [details] [review] [review] > Excise Tubes from channel.xml to fix build I've already pushed a fix for this, admittedly as a fixup! commit and then forgot to autosquash. Oh well. (In reply to comment #7) > Created attachment 56411 [details] [review] [review] > Remove deprecated, unused debug-ansi.h Haven't you forgotten to actually remove debug-ansi.h in this commit? (In reply to comment #8) > Created attachment 56412 [details] [review] [review] > Remove deprecated tp_message_new() You forgot to remove tp_message_new from telepathy-glib-sections.txt.
(In reply to comment #9) > (In reply to comment #7) > > Created attachment 56411 [details] [review] [review] [review] > > Remove deprecated, unused debug-ansi.h > > Haven't you forgotten to actually remove debug-ansi.h in this commit? And you forgot to remove all the debug-ansi stuff from the docs (at least -docs.sgml).
http://cgit.freedesktop.org/~jonny/telepathy-glib/log/?h=remove-deprecations Here is some more stuff that's been deprecated to remove.
... and some more fixes: (In reply to comment #5) > * Version should be < 1.0 at first (0.99.n where n is a snapshot number > incremented when it's halfway stable for a bit? 0.2012.n because this is the > end of the world? ...) and become 1.0 when the core library reaches API/ABI > stability I've kept this at 0.99.0 for now. > * "Presence Presence interface" Fixed. > * tp_message_to_text should still return whether there's non-text content What would you prefer? gboolean tp_message_to_text (TpMessage *message, gchar **text); or gchar * tp_message_to_text (TpMessage *message, gboolean *non_text_content); where the former returns TRUE if there *is* text content (i.e. @text is set), and the latter returns NULL and sets @non_text_content to TRUE if there *isn't* text content... I think the former is nicer. Also: > * Commented-out Call stuff would be better #ifdef'd? #ifdef'd on what?
(In reply to comment #11) > http://cgit.freedesktop.org/~jonny/telepathy-glib/log/?h=remove-deprecations > > Here is some more stuff that's been deprecated to remove. This branch looks ideal, please merge. (In reply to comment #12) > > * tp_message_to_text should still return whether there's non-text content > [...] returns TRUE if there *is* text content (i.e. @text is set), > and the latter returns NULL and sets @non_text_content to TRUE if there *isn't* > text content... I think the former is nicer. There's a subtle distinction between what I'm asking for and what you seem to be suggesting: the point of the NON_TEXT_CONTENT flag was not "there is no text here", it was "by condensing this Message into plain-text, you are losing something". Consider this composite message: text/plain: "hey look at this funny picture lol" image/jpeg: ... This would "text-ize" as: ("hey look at this funny picture lol", NON_TEXT_CONTENT) which a text-only UI could display as something like hey look at this funny picture lol [Non-text content not displayed, sorry!] Meanwhile, this message (which could be produced automatically by a sufficiently clever CM receiving only the HTML version): text/html: "look at this <a href="http://...">website</a>", alternative for AAAA text/plain: "look at this website <http://...>", alternative for AAAA would "text-ize" as: ("look at this website <http://...>", 0) which a text-only UI would display without the apology. I'm not sure how useful this would really be in practice, though... > > * Commented-out Call stuff would be better #ifdef'd? > > #ifdef'd on what? I don't know, #ifdef FIXME_REINSTATE_WHEN_CALL_IS_STABLE or something. Or you could leave them commented-out (or #if 0) and have an additional comment that's something distinctive we can grep for, or references a new bug that says "reinstate Call-related things in tp-glib when it's stable", or something. I do agree with you that it's a good idea not to follow the usual "don't comment out code" rule in this case.
http://cgit.freedesktop.org/~smcv/telepathy-glib/log?h=easy-deprecations supersedes the patches here (or it would if cgit was up). ssh://people.freedesktop.org/~smcv/telepathy-glib easy-deprecations
*** Bug 18832 has been marked as a duplicate of this bug. ***
(In reply to comment #14) > http://cgit.freedesktop.org/~smcv/telepathy-glib/log?h=easy-deprecations > supersedes the patches here (or it would if cgit was up). > > ssh://people.freedesktop.org/~smcv/telepathy-glib easy-deprecations This all looks good. I wonder if we should make TpBaseChannel check there are no duplications after calling get_interfaces?
http://cgit.freedesktop.org/~jonny/telepathy-glib/log/?h=moar-deprecations This: · #ifdefs out the Call stuff. I reckon we should start generating Call code soon. · replaces NUM_TP_… defines with TP_NUM_… · moves chat states to TpTextChannel and adds a chat states test.
(In reply to comment #17) > http://cgit.freedesktop.org/~jonny/telepathy-glib/log/?h=moar-deprecations > > This: > > · #ifdefs out the Call stuff. I reckon we should start generating Call code > soon. > > · moves chat states to TpTextChannel and adds a chat states test. I reviewed these on Bug #46470 - sorry, missed them here. > · replaces NUM_TP_… defines with TP_NUM_… Superseded by Bug #46470 which adds the new version on master, then deletes the old on next. I think that's generally a good thing to do, for things that won't cause massive bloat/effort?
(In reply to comment #16) > > ssh://people.freedesktop.org/~smcv/telepathy-glib easy-deprecations > > This all looks good. I merged everything except the last commit, which needs updating (there's now one more example). I'll shunt those onto a new bug I think. > I wonder if we should make TpBaseChannel check there are no duplications after > calling get_interfaces? That might not be a bad idea. Having said that, it'd be O(n**2) at the moment... so perhaps not worth it.
I've searched all open bug to see all those requesting API breaks and have set them as blocking this bug. I've also reported all things I could think. Now I'm pretty confident the "Depends on" list is complete... scary how long it is...
Xavier merged master into next and removed some deprecations here: http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=next-remove-deprecated Some comments: 02050c89a381c9e9e: You could use tp_asv_new (NULL, NULL) instead of g_hash_table_new_full (blah blah blah). cdb02889684c082: + test->sms_channel = TP_TEXT_CHANNEL (tp_client_factory_ensure_channel ( + factory, test->connection, chan_path, props, &test->error)); g_assert_no_error (test->error); + g_assert (TP_IS_TEXT_CHANNEL (test->channel)); The second assertion should be test->sms_channel, no? It's also redundant (there's another too) as the TP_TEXT_CHANNEL cast will typecheck, no? Otherwise, seems good.
Branch updated with more fixes.
(In reply to comment #22) > Branch updated with more fixes. Yes!
Merci !
Yet another branch to review: http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=next-update-spec
http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=next-update-spec&id=b78224495b798464759f1c7ae8bf4372dc0415b5 + self->priv->alias_flags = tp_asv_get_uint32 (properties, "AliasFlags", NULL); You should check valid and raise an error if !valid. Why are you changing the presence mixin in this commit? http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=next-update-spec&id=1dcdb3b38c99257a581c13861720770f5b4b38b4 looks good
(In reply to comment #26) > http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=next-update-spec&id=b78224495b798464759f1c7ae8bf4372dc0415b5 > > + self->priv->alias_flags = tp_asv_get_uint32 (properties, "AliasFlags", > NULL); > You should check valid and raise an error if !valid. done > Why are you changing the presence mixin in this commit? my mistake, splitted the commit Branch merged.
Remove deprecated stuff, again... http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=next-remove-deprecated
(In reply to comment #28) > Remove deprecated stuff, again... > http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=next- > remove-deprecated This branch doesn't exist; I guess it has been merged.
This issue has been moved to https://gitlab.freedesktop.org/telepathy/telepathy-glib/issues/48 in GitLab; please submit any further comments there.
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.