Review Request: Add a option to let the user go online as soon as the wizard exits
David Edmundson
kde at davidedmundson.co.uk
Tue Apr 5 16:15:56 CEST 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101031/#review2395
-----------------------------------------------------------
I think your code also shows the checkbox when you edit an existing account - and then the button doesn't do anything.
src/KCMTelepathyAccounts/account-edit-widget.h
<http://git.reviewboard.kde.org/r/101031/#comment2049>
Don't expose objects like!
make QCheckBox* private and have a getter method to fetch it.
Reason:
- you don't want other objects to be able to just playing around with your pointers all over the place.
---
Also I would have expected to just have a boolean method, as the calling code doesn't need to know that's it's stored in a checkbox.
bool connectOnAdd()
{
return m_connectOnAddCheckBox->isChecked();
}
src/KCMTelepathyAccounts/account-edit-widget.cpp
<http://git.reviewboard.kde.org/r/101031/#comment2050>
<QtGui/QCheckBox>
src/add-account-assistant.cpp
<http://git.reviewboard.kde.org/r/101031/#comment2048>
always use
if(something) {
} <- with the braces.
Even if it's just one line.
- David
On April 5, 2011, 2:09 p.m., Rohan Garg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101031/
> -----------------------------------------------------------
>
> (Updated April 5, 2011, 2:09 p.m.)
>
>
> Review request for Telepathy.
>
>
> Summary
> -------
>
> Add a option to let the user go online with his new account as soon as he adds it.
>
>
> This addresses bug 269911.
> http://bugs.kde.org/show_bug.cgi?id=269911
>
>
> Diffs
> -----
>
> src/KCMTelepathyAccounts/account-edit-widget.h 8e0c7ea
> src/KCMTelepathyAccounts/account-edit-widget.cpp 5c09e5d
> src/add-account-assistant.cpp e038e8b
>
> Diff: http://git.reviewboard.kde.org/r/101031/diff
>
>
> Testing
> -------
>
> Patch compiles and works as described above, i could see my account go online as soon as i added it, and when the tick box was unchecked, the account did not connect
>
>
> Screenshots
> -----------
>
>
> http://git.reviewboard.kde.org/r/101031/s/112/
>
>
> Thanks,
>
> Rohan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110405/ce5292fe/attachment.htm
More information about the KDE-Telepathy
mailing list