Review Request: Changed plugin structure and added convenience methods

[David Edmundson]

> On Jan. 13, 2011, 7 p.m., David Edmundson wrote:
> > src/KCMTelepathyAccounts/parameter-edit-model.cpp, line 72
> > <http://git.reviewboard.kde.org/r/100376/diff/1/?file=6760#file6760line72>
> >
> >     I'm not sure I like this.
> >     
> >     firstly it's better not to duplicate code
> >     
> >     case Qt::DisplayRole:
> >      /*Fall through*/
> >     case Qt::EditRole:
> >      //do stuff
> >      break;
> >     
> >     that way if you have to change one, it's fine.
> >     
> >     I don't like having EditRole, and ValueRole when they both do the same thing. We should just have one.
> 
> Thomas Richard wrote:
>     DisplayRole and EditRole are needed to use the QDataWidgetMapper. (Or at least one of them) We should get rid of ValueRole then..

The last part can be a future optimisation/cleanup.


- David


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


On Jan. 13, 2011, 6:05 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, 6:05 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/54d3e098/attachment.htm