[Bug 35341] Bind Connection.ContactBlocking

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Mar 15 22:01:33 CET 2011


https://bugs.freedesktop.org/show_bug.cgi?id=35341

--- Comment #2 from Andre Moreira Magalhaes <andrunko at gmail.com> 2011-03-15 14:01:33 PDT ---
So first things first. Let me explain a bit on how tp-qt4 deal with new new
D-Bus interfaces additions.

The files in the spec/ directory are the same files found in
telepathy-spec-<version>/spec for a given tp-spec version. In order to add new
files released in a new spec, we first add all files from that spec, generate
low-level bindings for all new interfaces (including adding pretty headers) and
only then we use the new low-level bindings.

Said that I would like some changes in your branch scheme:
- First create a branch called spec-unreleased that adds the file
spec/Connection_Interface_Contact_Blocking.xml, generates low-level bindings
for it (including adding the pretty header). (no change in existing code here)
- After that edit your first patch at your blocking branch, remove the parts
you already did in the branch spec-unreleased (above) and rebase the blocking
branch against spec-unreleased.

This way we can integrate your branch easily when we merge the branch to update
to the new spec (not your spec-unreleased branch, but a correct one adding
everything from the new spec when that is released) and no conflict (probably)
will happen from your blocking branch and current tp-qt4 master (which will
include the new spec additions).

Also when doing this, please rebase both of your branches on top of current
master.

Now for the code itself:
- In ContactManager::Roster::introspect() you don't want to introspect both the
deny channel and the ContactBlocking iface, use the ContactBlocking iface if
supported and the Deny Channel as fallback. You also want to call
requestBlockedContacts _async_ here and only finish FeatureRoster introspection
once you get the initial blocked contacts.

In resume:
Call introspectContactBlocking (adding the support to get the initial blocked
contacts) in ::introspect not in ::onContactListChannelReady() if
ContactBlocking is supported and do not call the deny channel introspection
code in this case. Once ContactBlocking finishes introspection, call
introspectContactList and you should be fine. Check the other code path for the
deny channel, you may need some other changes.

- Coding style. Add space after ",", if, etc, check HACKING
+        return queuedFinishVoid(iface->BlockContacts(handles,reportAbuse));
Also here:
+        if(hasContactBlockingInterface) {


- _never_ call blocking D-Bus code inside tp-qt4 as done in:
 +        UIntList handles = iface->RequestBlockedContacts();

- _never_ change public API signature as you did in
ContactManager::blockContacts, instead add a new overload method for this,
without any default arg.

- Either remove ContactManager::blockedContacts() or make it return a cached
list of blocked contacts (no D-Bus call).
  for i in allcontacts: if contact->isBlocked() ret << contact

That is it for now, I will check again once the branch is updated.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list