Review Request: Changed plugin structure and added convenience methods

David Edmundson kde at davidedmundson.co.uk
Thu Jan 13 21:55:12 CET 2011


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



src/KCMTelepathyAccounts/abstract-account-parameters-widget.h
<http://git.reviewboard.kde.org/r/100376/#comment686>

    const QList<QWidget*> &labelWidgets



src/KCMTelepathyAccounts/abstract-account-parameters-widget.h
<http://git.reviewboard.kde.org/r/100376/#comment687>

    be consistent with your 'QWidget* item' or 'QWidget *item'
    
    I think KDE coding standard is the second one.



src/KCMTelepathyAccounts/abstract-account-parameters-widget.cpp
<http://git.reviewboard.kde.org/r/100376/#comment688>

    if (...) {
    
    }



src/KCMTelepathyAccounts/abstract-account-parameters-widget.cpp
<http://git.reviewboard.kde.org/r/100376/#comment689>

    You've lost your check of parameterType...



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

    if (...) {
    
    }
    
    always have the braces.



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

    Not your fault, but 
    
    if ( ) {
    ...
    }



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

    Don't really need the two breaks in here.
    get rid of the first break and the word "else"



src/KCMTelepathyAccounts/parameter-edit-model.cpp
<http://git.reviewboard.kde.org/r/100376/#comment692>

    imho it's still clearer to just to:
    
     data = QVariant(m_items.at(index.row())->parameter().defaultValue());
    
    than to call the data() again.



src/KCMTelepathyAccounts/parameter-edit-model.cpp
<http://git.reviewboard.kde.org/r/100376/#comment695>

    braces


- David


On Jan. 13, 2011, 8:39 p.m., Dominik Schmidt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100376/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2011, 8:39 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This got a little huge, I'm sorry.
> 
> This patch refactors the way plugins work.
> It uses (kind of) the methods from the last review request but additionally uses the QDataWidgetMapper as suggested (and partly implemented) by Thomas - please tell me your mail address!
> 
> Also it avoids passing around ParameterLists and Values and instead passes the ParameterEditModel through all levels.
> 
> 
> Diffs
> -----
> 
>   src/KCMTelepathyAccounts/CMakeLists.txt 082f3352c4e5855d87b594620e7e317160b557c1 
>   src/KCMTelepathyAccounts/abstract-account-parameters-widget.h 44bba673ee0479f7253e6c2ac0860888362d0642 
>   src/KCMTelepathyAccounts/abstract-account-parameters-widget.cpp d0ad71a3660ed2514817a67e6fd2a283481b54d9 
>   src/KCMTelepathyAccounts/abstract-account-ui.h a90395ca4e429fa37ccf935f19fcd0063397d4e9 
>   src/KCMTelepathyAccounts/abstract-account-ui.cpp a2a09c0485e2eec5e64c96a91ddaddea2747e68d 
>   src/KCMTelepathyAccounts/account-edit-widget.h 9e8ac689eb15d7666a3be06f98af5a67703efd6c 
>   src/KCMTelepathyAccounts/account-edit-widget.cpp d78cf93fbd9bb905ef2b382e07b0ac780f2f9e3b 
>   src/KCMTelepathyAccounts/generic-advanced-options-widget.h PRE-CREATION 
>   src/KCMTelepathyAccounts/generic-advanced-options-widget.cpp PRE-CREATION 
>   src/KCMTelepathyAccounts/include/GenericAdvancedOptionsWidget PRE-CREATION 
>   src/KCMTelepathyAccounts/include/ParameterEditModel PRE-CREATION 
>   src/KCMTelepathyAccounts/parameter-edit-model.h 4eac5a7512c8e0a1f90f7d570f69d0659b61c2b6 
>   src/KCMTelepathyAccounts/parameter-edit-model.cpp a30481ef12ef11533aab26b3200c94eb50139f49 
>   src/KCMTelepathyAccounts/parameter-edit-widget.h 566cbe1227adb7e0bd7ee4637f1263273f83333b 
>   src/KCMTelepathyAccounts/parameter-edit-widget.cpp 7dd1e530d167a860564200351dea5ae3cd00f9a8 
>   src/KCMTelepathyAccounts/parameter-item.cpp 8f907f7988b10d0e58f5beddf04ebedd54fdfb00 
>   src/add-account-assistant.cpp 184cf93ef8c7b1df944c3fb54cde8f5b6749cec2 
>   src/edit-account-dialog.cpp 45ec8407f943da77175d6f320fe13b92b75874f1 
> 
> Diff: http://git.reviewboard.kde.org/r/100376/diff
> 
> 
> Testing
> -------
> 
> Yes, works for me for adding and editing accounts.
> 
> 
> Thanks,
> 
> Dominik
> 
>

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


More information about the KDE-Telepathy mailing list