Review Request: Compile TP Account KCM against Tp-Qt4 0.5

Dario Freddi drf at kde.org
Thu Jan 6 17:32:06 CET 2011


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


Almost there, some coding style issue and IMHO one big thing to revisit.


src/KCMTelepathyAccounts/account-edit-widget.cpp
<http://git.reviewboard.kde.org/r/100293/#comment567>

    Code style, the same as above



src/KCMTelepathyAccounts/protocol-item.cpp
<http://git.reviewboard.kde.org/r/100293/#comment568>

    Brackets: if () {



src/KCMTelepathyAccounts/protocol-parameter-value.h
<http://git.reviewboard.kde.org/r/100293/#comment571>

    Given what you are doing here, I would rather use QSharedData for this class' private member. It is more efficient and it looks exactly what you are looking after. Feel free to ask some guidance if you never used this technique before.



src/KCMTelepathyAccounts/protocol-parameter-value.cpp
<http://git.reviewboard.kde.org/r/100293/#comment569>

    Code style: avoid spaces: if (!d->parameter...
    
    This applies to all of those occurrences



src/add-account-assistant.cpp
<http://git.reviewboard.kde.org/r/100293/#comment565>

    Code style: foreach (...) {



src/edit-account-dialog.cpp
<http://git.reviewboard.kde.org/r/100293/#comment566>

    Code style, as above.


- Dario


On 2011-01-05 23:32:36, David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100293/
> -----------------------------------------------------------
> 
> (Updated 2011-01-05 23:32:36)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
>     Compile against Telepathy-Qt4 0.5
>     Changed account abstract-account-parameters-widget.h to return
>     QVariantMap of parameterValues() instead of trying to
>     QMap<ParameterItem,QString> which didn't compile.
>     
>     There didn't appear to be a reason for doing this as it was just copied
>     to a QVariantMap directly after anyway.
>     
>     Note that the code now doesn't remove any parameters which have the same
>     value as the default. This didn't seem to be a step required by
>     Telepathy.
>     
>     Also updated parameter-edit-model to always return the value in the type
>     requested by the ProtocolParameter.
> 
> 
> Diffs
> -----
> 
>   src/KCMTelepathyAccounts/CMakeLists.txt a74ae27c89e1ad9e0ad6948bf959f4c93bd1ab7c 
>   src/KCMTelepathyAccounts/abstract-account-parameters-widget.h 6357b4eaff3b5579b44d9040ffe33314bbf09d80 
>   src/KCMTelepathyAccounts/account-edit-widget.h 4c78338603dc0b88230b52190ca9d373dff2628d 
>   src/KCMTelepathyAccounts/account-edit-widget.cpp b554db71a17a4c9664d04f781561ce62cbc17562 
>   src/KCMTelepathyAccounts/parameter-edit-model.h 0c762ea5a30253cd1c4b772f3859b681f4fa8dd4 
>   src/KCMTelepathyAccounts/parameter-edit-model.cpp 0fe52a10798b03dff5491d78c1b16ad72e2f0533 
>   src/KCMTelepathyAccounts/parameter-edit-widget.h 4d3b5a62735bd519ce01f39037dad151e6b34cb3 
>   src/KCMTelepathyAccounts/parameter-edit-widget.cpp 187c39fbf990b610b8985d1869d48bc2be7c9c6d 
>   src/KCMTelepathyAccounts/parameter-item.h 14b11160204a5b76ee3b0fed65861d0ceec87a7f 
>   src/KCMTelepathyAccounts/parameter-item.cpp 318f175ec60b0dc31edadb72eda769b791ff9827 
>   src/KCMTelepathyAccounts/protocol-item.cpp 03ac0bceff0f32673a706329f698a66323842c69 
>   src/KCMTelepathyAccounts/protocol-parameter-value.h PRE-CREATION 
>   src/KCMTelepathyAccounts/protocol-parameter-value.cpp PRE-CREATION 
>   src/account-item.cpp 50afaa54c96ad51b6775f954cc180d1e12bbe96b 
>   src/add-account-assistant.cpp 3933763957b1448aacf9e6c7efbc6b296a5e3664 
>   src/edit-account-dialog.cpp 73462f2e2a93e22e6d5d67297c575f4e10d44c5c 
> 
> Diff: http://git.reviewboard.kde.org/r/100293/diff
> 
> 
> Testing
> -------
> 
> Checked it now compiled
> Added an account, edited an account and removed an account. Account adding and editing done using both the UI plugins and the generic configuration.
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110106/bbf5ad89/attachment-0001.htm 


More information about the KDE-Telepathy mailing list