-----------------------------------------------------------
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
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to