Review Request: Synchronize tp-integration-daemon with subscription/publication states and newly added contacts
Dario Freddi
drf54321 at gmail.com
Mon Apr 5 19:03:52 CEST 2010
> On 2010-04-05 14:41:10, George Goldberg wrote:
> > In general, this all looks fine to me (usual disclaimer - I haven't compiled or tested it yet).
> >
> > The one area that seems to be a bit problematic is that when the TelepathyContact constructor is run, and doNepomukSetup() method is called. I see that you call the slots to handle subscription state and blocked status after calling doNepomukSetup(). Could perhaps some of the redundant code in doNepomukSetup() (at least the bit creating the isBuddyOf relationship) be removed?
Good catch - it probably is. I'll check it out later.
> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 325
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line325>
> >
> > I'm not sure what the possibilities are here. Can you explain please?
Basically - do we want to record if the contact is no longer subscribed to us? Random example: in MSN, if a contact unsubscribes from you but still does not block you, you can see in your client (if it supports it, like KMess) "This contact has no longer you in his contact list".
> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 355
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line355>
> >
> > Is there a bug report against Nepomuk for that? Or is it not as simple as that?
I don't know - I really wanted to check as usual if it was me doing something totally wrong or not.
> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 364
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line364>
> >
> > Umm, I'm not sure, for now. Let's leave a FIXME there and figure out what to do later.
> >
> > It'll probably be some way for the UI to know the subscription state is ASK because it'll want to have some "resend subscription request" button.
Ok - in this case we probably also have to notify the UI about capabilities (are you able to cancel a subscription request, etc).
> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 91
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line91>
> >
> > m_contact->publishState() would do I think.
> >
> > Same applies to the two lines below.
Are we using this as "coding style"? Here I'm following the latest trend in Qt API, aka QWeakPointer, where operator-> is no longer defined and you explicitely have to access the pointer using .data(). I can switch back using -> though.
- Dario
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3468/#review4876
-----------------------------------------------------------
On 2010-03-31 20:16:58, Dario Freddi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3468/
> -----------------------------------------------------------
>
> (Updated 2010-03-31 20:16:58)
>
>
> Review request for telepathy and George Goldberg.
>
>
> Summary
> -------
>
> This patch makes tp-integration-daemon pick up the changes done at a subscription/publication level to telepathy contacts. Bottom line: now adding/removing contacts from tp-contactlist no longer screws up your nepomuk database.
>
> Please note that tp-contactlist won't pick up changes in realtime: you have to restart it to see the effect. This is because tp-contactlist does not watch nepomuk resources for changes (my next target, after this patch and the other one will make it in)
>
> BEWARE: this patch is dependent on another patch (eww) to Tp-Qt4, which lies in fd.o bug 27379, and will hopefully make it in before this patch will get merged.
>
> Blocking contacts will come in a further revision, since I need some ontology changes.
>
>
> Diffs
> -----
>
> /trunk/playground/network/telepathy-integration-daemon/telepathy.trig 1109698
> /trunk/playground/network/telepathy-integration-daemon/telepathyaccount.h 1109698
> /trunk/playground/network/telepathy-integration-daemon/telepathyaccount.cpp 1109698
> /trunk/playground/network/telepathy-integration-daemon/telepathycontact.h 1109698
> /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp 1109698
>
> Diff: http://reviewboard.kde.org/r/3468/diff
>
>
> Testing
> -------
>
> Works, as shown in tp-contactlist.
>
>
> Thanks,
>
> Dario
>
>
More information about the KDE-Telepathy
mailing list