Review Request: Move accountManagerPtr from config to main applet class

[David Edmundson]

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



src/config.cpp
<http://git.reviewboard.kde.org/r/102762/#comment6200>

    Why not set parents on these? Then you don't have to delete them.



src/telepathy-contact.cpp
<http://git.reviewboard.kde.org/r/102762/#comment6195>

    You don't need any of these except FeatureCore



src/telepathy-contact.cpp
<http://git.reviewboard.kde.org/r/102762/#comment6196>

    You don't need any of these (not sure if you still need FeatureCore)



src/telepathy-contact.cpp
<http://git.reviewboard.kde.org/r/102762/#comment6198>

    you might want to wrap this entire method in 
    
    if (!isUserConfiguring()){
    
    }
    
    to avoid ever displaying two configs.
    (unless plasma does that for you before calling your showConfigurationInterface)



src/telepathy-contact.cpp
<http://git.reviewboard.kde.org/r/102762/#comment6197>

    After the first run the config still exists until you start another config, and m_config will always be non 0.
    
    Simpler code, let it delete itself: 
    
    Config *config = new Config(...)
    connect(config, SIGNAL(newContactStuff...
    connect(config, SIGNAL(accepted()), config, SLOT(deleteLater());
    config->show();
    
    Then it will just delete itself when it's closed, which is what you want.



src/telepathy-contact.cpp
<http://git.reviewboard.kde.org/r/102762/#comment6199>

    Additional note: never call "delete" on a QObject.
    
    Either use object->deleteLater();
    (which is less crash prone)
    
    or set a parent object so it can delete itself nicely.


- David Edmundson


On Oct. 3, 2011, 11:17 a.m., Francesco Nwokeka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102762/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2011, 11:17 a.m.)
> 
> 
> Review request for Telepathy and David Edmundson.
> 
> 
> Description
> -------
> 
> Moving the contactManagerPtr from the config class to the main applet lightens the applet. This because we don't keep track of every change made in the Tp model by creating the config object only when needed. Thus the models for display too.
> 
> 
> Diffs
> -----
> 
>   src/config.h 6d3e27d 
>   src/config.cpp 9734a83 
>   src/telepathy-contact.h ce7632f 
>   src/telepathy-contact.cpp 34ce6db 
> 
> Diff: http://git.reviewboard.kde.org/r/102762/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Francesco Nwokeka
> 
>

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