Review Request: KWallet::Wallet::openWallet or will it?

[George Kiagiadakis]

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

Ship it!


I believe this is ok to ship after fixing a few small issues...


KTp/pending-wallet-operation.h
<http://git.reviewboard.kde.org/r/105566/#comment13231>

    Nitpick here, I think "PendingWallet" or "PendingWalletInterface" would be a better name for this class. "PendingWalletOperation" sounds like a class that does some operation on the wallet, not a class that returns a wallet.
    
    tp-qt uses similar naming... PendingAccount, PendingConnection, ...



KTp/pending-wallet-operation.h
<http://git.reviewboard.kde.org/r/105566/#comment13233>

    This constructor should be private imho and the whole class should be a friend with WalletInterface, so that you can construct it only from there.



KTp/pending-wallet-operation.h
<http://git.reviewboard.kde.org/r/105566/#comment13232>

    make that a const method



KTp/pending-wallet-operation.cpp
<http://git.reviewboard.kde.org/r/105566/#comment13230>

    You need a destructor to do "delete d;"



KTp/wallet-interface.h
<http://git.reviewboard.kde.org/r/105566/#comment13236>

    I would also make this destructor private, since the user is not supposed to delete this class.



KTp/wallet-interface.cpp
<http://git.reviewboard.kde.org/r/105566/#comment13235>

    delete d;


- George Kiagiadakis


On July 14, 2012, 3:08 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105566/
> -----------------------------------------------------------
> 
> (Updated July 14, 2012, 3:08 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Adds a method to asynchronously open the wallet. This must be called (and completed) by the application if they want to use any other methods.
> API is a bit horrendous, but that's mostly because of KWallet.
> 
> Wallet opening appears as a Tp::PendingOperation so that it can be used in a composite job.
> 
> 
> Diffs
> -----
> 
>   KTp/CMakeLists.txt 61da66d 
>   KTp/pending-wallet-operation.h PRE-CREATION 
>   KTp/pending-wallet-operation.cpp PRE-CREATION 
>   KTp/wallet-interface.h c43fb38 
>   KTp/wallet-interface.cpp d2dc2cb 
> 
> Diff: http://git.reviewboard.kde.org/r/105566/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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