Review Request: Add syntax for changing global presence

[David Edmundson]

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



src/contactrunner.cpp
<http://git.reviewboard.kde.org/r/105407/#comment11983>

    The reason we have setAccountManager is so that you can create the globalPresence object at initialisation. 
    
    Move the "m_globalPresence = new KTp::GlobalPresence()" to the ctor.
    
    Same for AccountsModel.
    Then you can remove your destructor, and remove your checks everywhere. 



src/contactrunner.cpp
<http://git.reviewboard.kde.org/r/105407/#comment11986>

    We can't keep filling the code with "mid(3)" everywhere. 
    
    Magic numbers are always bad and it's impossible to read and harder to debug.
    
    Consider .split("whatever");
    or QString("whatever").length();
    or .startsWith("whatever");
    
    What is the string we're splitting?
    
    I don't care if it's slower if it's easier to read.



src/contactrunner.cpp
<http://git.reviewboard.kde.org/r/105407/#comment11987>

    Use KTp::Presence() to get the icon. (you create one anyway).
    
    Also KIcon("blah") is generally used instead of QIcon::fromTheme("blah"). 



src/contactrunner.cpp
<http://git.reviewboard.kde.org/r/105407/#comment11989>

    


- David Edmundson


On July 2, 2012, 8:22 a.m., Dan Vratil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105407/
> -----------------------------------------------------------
> 
> (Updated July 2, 2012, 8:22 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> This patch adds syntax for changing global presence:
> 
> go online|away|busy|hidden|offline [status message]
> 
> The status message is optional. The "go" keyword and presence names are translatable.
> 
> 
> Diffs
> -----
> 
>   src/contactrunner.h c316aa4 
>   src/contactrunner.cpp 0d48c79 
> 
> Diff: http://git.reviewboard.kde.org/r/105407/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vratil
> 
>

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