Review Request 126834: ktp-common-internals propagating presence model for status handler integration and global / multiple account presence fixes

[David Edmundson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126834/#review93493
-----------------------------------------------------------



General approach seems like a good idea.

You have one big bug in here (comment #2).


KTp/Models/presence-model.cpp (line 85)
<https://git.reviewboard.kde.org/r/126834/#comment63745>

    you should be able to have 
    presenceModelChanged(KTp::Presence, bool)
    
    and have it automatically cast as long as you have registered KTp::Presence as a DBus type



KTp/Models/presence-model.cpp (line 184)
<https://git.reviewboard.kde.org/r/126834/#comment63747>

    on the initial load we call addPresence; addPresence is emitting DBus signals to all the other clients when it's not actually adding anything, but just loading the existing ones which all the other clients should already have.
    
    It won't break as the other clients won't do anything, but it's rather wasteful.



KTp/Models/presence-model.cpp (line 230)
<https://git.reviewboard.kde.org/r/126834/#comment63746>

    Personally I'd use two different DBus signals for add/remove.


- David Edmundson


On Feb. 23, 2016, 7:16 a.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126834/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2016, 7:16 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> -------
> 
> New features:
> -Simplified API, sends and collects changes over DBus.
> 
> 
> Diffs
> -----
> 
>   KTp/Models/presence-model.h 8f206b880f48640626322269a14956f105482f69 
>   KTp/Models/presence-model.cpp ddc1a7c75f1a452bf3ac2db1aecbd88a5d1ce519 
> 
> Diff: https://git.reviewboard.kde.org/r/126834/diff/
> 
> 
> Testing
> -------
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20160313/7f3e9c20/attachment-0001.html>