Review Request 111256: Import the KPeople KTP integration plugins

David Edmundson david at davidedmundson.co.uk
Wed Jun 26 21:16:14 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111256/#review35126
-----------------------------------------------------------


Self review.



kpeople/actionsplugin/implugin.h
<http://git.reviewboard.kde.org/r/111256/#comment25695>

    ImPlugin is a fairly generic name.
    Not good when we have 2 :D
    
    KPeopleIMActions plugin ?



kpeople/actionsplugin/implugin.cpp
<http://git.reviewboard.kde.org/r/111256/#comment25691>

    Wrong brackets



kpeople/common/CMakeLists.txt
<http://git.reviewboard.kde.org/r/111256/#comment25696>

    Having done this, just putting this in the k-c-i main directory makes more sense. Right?
    
    I did it to avoid it being part of the lib, but I don't think that would cause any problems.
    



kpeople/common/impersonsdatasource.h
<http://git.reviewboard.kde.org/r/111256/#comment25692>

    wrong brackets



kpeople/common/impersonsdatasource.h
<http://git.reviewboard.kde.org/r/111256/#comment25693>

    Not needed



kpeople/common/impersonsdatasource.cpp
<http://git.reviewboard.kde.org/r/111256/#comment25694>

    wrong brackets



kpeople/common/impersonsdatasource.cpp
<http://git.reviewboard.kde.org/r/111256/#comment25697>

    Turns out this isn't ideal, if I do "group by accounts" whilst one of my accounts is offline, everything freezes for quite some time.
    
    I have a cunning plan on how to fix this:
     - we create a new PersonsModelFeature that includes the IM accountID as a new model role, using most of this query here. This will be in KTp code only, not in libkpeople as it is very app-specific
    
     - We then have an accountID which we can use in KTpTranslationProxyModel for KTp::AccountRole.
    
    Not too important to discuss here.
    
    



kpeople/common/impersonsdatasource.cpp
<http://git.reviewboard.kde.org/r/111256/#comment25698>

    FWIW, I think the reason we do objectPath is so we could do
    
    Tp::AccountPtr = Tp::Account::create(objectPath);
    
    rather than going through the AM.


- David Edmundson


On June 26, 2013, 2:33 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111256/
> -----------------------------------------------------------
> 
> (Updated June 26, 2013, 2:33 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Import the KPeople KTP integration plugins
> 
> There are two plugins, one for the datasource which provides
> IM data in real time.
> 
> The other is an actions plugin which provides actions that can be started on a PersonsData.
> 
> We need to provide KTp integration into libkpeople in a plugin, but we also want to access this
> singleton of accounts/contacts from the other plugin and from the KTp Models. To do this a static library
> ktpkpeople is created and everything links against that.
> 
> KTp Models the actions plugin and the im data source all link against this.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt a83069d59fed945d04560662e3a307ef33b28332 
>   KTp/Models/CMakeLists.txt 720a5b02465b825b08d12d7c43342ad1ba69c9d1 
>   kpeople/actionsplugin/CMakeLists.txt PRE-CREATION 
>   kpeople/actionsplugin/implugin.h PRE-CREATION 
>   kpeople/actionsplugin/implugin.cpp PRE-CREATION 
>   kpeople/actionsplugin/ktp_kpeople_plugin.desktop PRE-CREATION 
>   kpeople/common/CMakeLists.txt PRE-CREATION 
>   kpeople/common/impersonsdatasource.h PRE-CREATION 
>   kpeople/common/impersonsdatasource.cpp PRE-CREATION 
>   kpeople/datasourceplugin/CMakeLists.txt PRE-CREATION 
>   kpeople/datasourceplugin/im_persons_data_source_plugin.desktop PRE-CREATION 
>   kpeople/datasourceplugin/impersonsdatasourceplugin.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111256/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


More information about the KDE-Telepathy mailing list