Review Request: Changed plugin structure and added convenience methods

[Dominik Schmidt]

> On Jan. 13, 2011, 7 p.m., David Edmundson wrote:
> > src/KCMTelepathyAccounts/parameter-edit-model.cpp, line 91
> > <http://git.reviewboard.kde.org/r/100376/diff/1/?file=6760#file6760line91>
> >
> >     It'd be quicker and easier to just call
> >     data = QVariant(m_items.at(index.row())->defaultValue());

Yes, but this way I take advantage of the already done implementation and if we want to change that someday (or have our own defaults in the model or something) it's already in there. So I'd like to keep it this way.


> On Jan. 13, 2011, 7 p.m., David Edmundson wrote:
> > src/KCMTelepathyAccounts/parameter-edit-model.cpp, line 94
> > <http://git.reviewboard.kde.org/r/100376/diff/1/?file=6760#file6760line94>
> >
> >     If you implement the above comment you don't need to do this.
> 
> Dominik Schmidt wrote:
>     Yes, but can't hurt to have it accessible whenever the model is around.
>     Maybe for a tooltip or whatever.
>     
>     If you dont like it at all, I'll remove it.

Changed my mind :P
See comment above.


> On Jan. 13, 2011, 7 p.m., David Edmundson wrote:
> > src/add-account-assistant.cpp, line 141
> > <http://git.reviewboard.kde.org/r/100376/diff/1/?file=6764#file6764line141>
> >
> >     call your variable parameterModel or something indicating what it is a model of. 
> >     
> >     Right now it's like having a variable called "string" or "number".

Yeah, me likes. :D


> On Jan. 13, 2011, 7 p.m., David Edmundson wrote:
> > src/KCMTelepathyAccounts/parameter-edit-model.cpp, line 159
> > <http://git.reviewboard.kde.org/r/100376/diff/1/?file=6760#file6760line159>
> >
> >     simply:
> >     
> >     return index(i);

Nope, column has to be set as well.
If we aim for the row, we should return column == 0.


- Dominik


-----------------------------------------------------------
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/1efa39e0/attachment-0001.htm