Review Request 115233: libKPeople: Draft version of SkypeDataSource

[Martin Klapetek]

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



src/plugins/skype/skypedatasource.cpp
<https://git.reviewboard.kde.org/r/115233/#comment34077>

    for static values I'd suggest either prepending them with s_ or make them ALL_CAPS



src/plugins/skype/skypedatasource.cpp
<https://git.reviewboard.kde.org/r/115233/#comment34076>

    I'm a bit worried about providing exact filename/relative filepath...if there are different filenames between versions or if skype decides to change this anytime, we're screwed.
    
    I'd suggest to use more heuristic approach here (I know this is just draft, just thinking ahead).
    
    Also what if you have more accounts in there? I assume that will simply process all of them? Then I have a scenario in my head - you are signed in skype with account1, you click on a contact from account2 - what will happen now? Should it (re-)log in the second account? Should it not care? I think we should store the skype-account-name as part of the contact ID like we do in KTp plugin, then just have the actions plugin decode and decide.



src/plugins/skype/skypedatasource.cpp
<https://git.reviewboard.kde.org/r/115233/#comment34075>

    This should close/delete m_db here


- Martin Klapetek


On Jan. 22, 2014, 6:19 p.m., Alexandr Akulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115233/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 6:19 p.m.)
> 
> 
> Review request for Telepathy, David Edmundson and Martin Klapetek.
> 
> 
> Repository: libkpeople
> 
> 
> Description
> -------
> 
> Initial version of SkypeDataSource plugin.
> UI for account selection is not yet implemented, so in current state plugin doesn't works. (You can edit skypedatasource.cpp:67 and manualy set your account)
> 
> 
> Diffs
> -----
> 
>   src/plugins/CMakeLists.txt 3633676 
>   src/plugins/skype/CMakeLists.txt PRE-CREATION 
>   src/plugins/skype/skype_kpeople_plugin.desktop PRE-CREATION 
>   src/plugins/skype/skypedatasource.h PRE-CREATION 
>   src/plugins/skype/skypedatasource.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/115233/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexandr Akulich
> 
>

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