Bug 69194 - stop Gabble using things deprecated in 0.20, 0.22
Summary: stop Gabble using things deprecated in 0.20, 0.22
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2013-09-10 18:53 UTC by Simon McVittie
Modified: 2013-09-12 10:38 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Cache PEP aliases in a hash table, not deprecated handle qdata (6.04 KB, patch)
2013-09-10 18:54 UTC, Simon McVittie
Details | Splinter Review
Cache vCard aliases in a hash table, not in deprecated handle qdata (5.05 KB, patch)
2013-09-10 18:54 UTC, Simon McVittie
Details | Splinter Review
exec-with-log.sh.in: fix out-of-tree use of GABBLE_TEST_BACKTRACE (958 bytes, patch)
2013-09-10 18:55 UTC, Simon McVittie
Details | Splinter Review
[1/3] GabbleFtManager: announce incoming FT channels individually (4.73 KB, patch)
2013-09-11 13:17 UTC, Simon McVittie
Details | Splinter Review
GabbleMucFactory: announce tube channels individually (3.27 KB, patch)
2013-09-11 13:18 UTC, Simon McVittie
Details | Splinter Review
Disable deprecated functionality (999 bytes, patch)
2013-09-11 13:19 UTC, Simon McVittie
Details | Splinter Review
Require tp-glib 0.21.1 and disable 0.22-deprecated functionality (1.22 KB, patch)
2013-09-11 17:47 UTC, Simon McVittie
Details | Splinter Review
regression tests: avoid relying on ye olde Capabilities (29.01 KB, patch)
2013-09-11 17:47 UTC, Simon McVittie
Details | Splinter Review
Remove support for ye olde Capabilities (45.77 KB, patch)
2013-09-11 17:50 UTC, Simon McVittie
Details | Splinter Review
caps/jingle-caps.py: convert test_prefer_phones to Call1 (5.38 KB, patch)
2013-09-11 18:06 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2013-09-10 18:53:52 UTC
Subject says it all. The patches I'm about to attach are necessary but not sufficient.
Comment 1 Simon McVittie 2013-09-10 18:54:36 UTC
Created attachment 85582 [details] [review]
Cache PEP aliases in a hash table, not deprecated handle qdata
Comment 2 Simon McVittie 2013-09-10 18:54:46 UTC
Created attachment 85583 [details] [review]
Cache vCard aliases in a hash table, not in deprecated  handle qdata
Comment 3 Simon McVittie 2013-09-10 18:55:22 UTC
Created attachment 85584 [details] [review]
exec-with-log.sh.in: fix out-of-tree use of GABBLE_TEST_BACKTRACE

---

commit name needs amending to what I said here
Comment 4 Simon McVittie 2013-09-10 18:56:17 UTC
I also need to amend the GabbleFtManager and GabbleMucFactory to announce FTs/tubes individually rather than as a bundle; that's easy, but making the tests cope is harder.
Comment 5 Xavier Claessens 2013-09-10 19:22:14 UTC
Comment on attachment 85583 [details] [review]
Cache vCard aliases in a hash table, not in deprecated  handle qdata

Review of attachment 85583 [details] [review]:
-----------------------------------------------------------------

::: src/vcard-manager.c
@@ +1681,5 @@
>    g_return_val_if_fail (tp_handle_is_valid (contact_repo, handle, NULL),
>        FALSE);
>  
> +  /* Return TRUE for positively or negatively cached contacts. */
> +  return g_hash_table_lookup_extended (priv->alias_cache,

g_hash_table_contains() is made for that.
Comment 6 Xavier Claessens 2013-09-10 19:22:48 UTC
+1 for the rest.
Comment 7 Simon McVittie 2013-09-11 13:17:58 UTC
Created attachment 85632 [details] [review]
[1/3] GabbleFtManager: announce incoming FT channels  individually

tp_channel_manager_emit_new_channels() is deprecated, because
announcing bundles of related channels together seemed a nice idea, but
it turns out to be really difficult to deal with in practice.

This requires some interesting contortions in test-multift.py, which
exercised the "bundle of related channels" stuff.

---

I applied the other patches, amending Atachment #85583 to use g_hash_table_contains() like you suggested.
Comment 8 Simon McVittie 2013-09-11 13:18:17 UTC
Created attachment 85633 [details] [review]
GabbleMucFactory: announce tube channels individually

tp_channel_manager_emit_new_channels() is deprecated, because
announcing bundles of related channels together seemed a nice idea, but
it turns out to be really difficult to deal with in practice.

We preserve the order in which we did things, as much as possible:
the text channel is announced (if it's going to be) before any of the
tubes.
Comment 9 Simon McVittie 2013-09-11 13:19:21 UTC
Created attachment 85634 [details] [review]
Disable deprecated functionality

This might regress if we deprecate more in telepathy-glib 0.21, but
still seems worth doing.
Comment 10 Guillaume Desmottes 2013-09-11 14:08:33 UTC
Comment on attachment 85632 [details] [review]
[1/3] GabbleFtManager: announce incoming FT channels  individually

Review of attachment 85632 [details] [review]:
-----------------------------------------------------------------

++
Comment 11 Guillaume Desmottes 2013-09-11 14:09:07 UTC
Comment on attachment 85633 [details] [review]
GabbleMucFactory: announce tube channels individually

Review of attachment 85633 [details] [review]:
-----------------------------------------------------------------

++
Comment 12 Guillaume Desmottes 2013-09-11 14:09:54 UTC
Comment on attachment 85634 [details] [review]
Disable deprecated functionality

Review of attachment 85634 [details] [review]:
-----------------------------------------------------------------

Shouldn't you bump the tp-glib dep as well?
Comment 13 Simon McVittie 2013-09-11 17:45:22 UTC
(In reply to comment #12)
> Comment on attachment 85634 [details] [review]
> Disable deprecated functionality
...
> Shouldn't you bump the tp-glib dep as well?

Yeah, I suppose so. Actually, I should have set the version to 0.22 (with required version actually 0.21.1) straight away - the commit message assumed I had, but I forgot to amend.
Comment 14 Simon McVittie 2013-09-11 17:47:29 UTC
Created attachment 85646 [details] [review]
Require tp-glib 0.21.1 and disable 0.22-deprecated  functionality

This might regress if we deprecate more in later telepathy-glib 0.21.x
versions, but still seems worth doing.
Comment 15 Simon McVittie 2013-09-11 17:47:49 UTC
Created attachment 85647 [details] [review]
regression tests: avoid relying on ye olde Capabilities

Telepathy 1.0 won't have these, only ContactCapabilities. We should
make sure that deleting Capabilities isn't a test-coverage regression.
Comment 16 Simon McVittie 2013-09-11 17:50:19 UTC
Created attachment 85650 [details] [review]
Remove support for ye olde Capabilities

---

caps/advertise-legacy.py is 100% legacy Capabilities, so I deleted it - advertise-contact-caps.py is the modern equivalent.

Everything else is OK, I just deleted the old version of the "expectations".
Comment 17 Simon McVittie 2013-09-11 18:06:05 UTC
Created attachment 85657 [details] [review]
caps/jingle-caps.py: convert test_prefer_phones to Call1

---

The Jingle tests in jingle/ all need converting to Call too, before we can drop StreamedMedia, but I've only made slow progress on that. This is a start...
Comment 18 Guillaume Desmottes 2013-09-12 07:52:47 UTC
Comment on attachment 85646 [details] [review]
Require tp-glib 0.21.1 and disable 0.22-deprecated  functionality

Review of attachment 85646 [details] [review]:
-----------------------------------------------------------------

++
Comment 19 Guillaume Desmottes 2013-09-12 07:53:49 UTC
Comment on attachment 85647 [details] [review]
regression tests: avoid relying on ye olde Capabilities

Review of attachment 85647 [details] [review]:
-----------------------------------------------------------------

++
Comment 20 Guillaume Desmottes 2013-09-12 07:55:35 UTC
Comment on attachment 85650 [details] [review]
Remove support for ye olde Capabilities

Review of attachment 85650 [details] [review]:
-----------------------------------------------------------------

++
Comment 21 Guillaume Desmottes 2013-09-12 07:56:07 UTC
Comment on attachment 85657 [details] [review]
caps/jingle-caps.py: convert test_prefer_phones to Call1

Review of attachment 85657 [details] [review]:
-----------------------------------------------------------------

++
Comment 22 Simon McVittie 2013-09-12 10:38:18 UTC
Merged for 0.19.0, thanks. I'll open a separate bug for deletion of StreamedMedia from the tests.


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.