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

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

Reply via email to