Review Request 114034: Bug 326078 - "Register new account" should not be available when editing jabber accounts

[David Edmundson]

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

Ship it!


Two minor comments for future patches.


plugins/gabble/main-options-widget.cpp
<http://git.reviewboard.kde.org/r/114034/#comment31674>

    Nitpick: having extra spaces causes problems.
    The next time someone else edits this file it will show they edited this file.
    
    If you use kate/kdevelop you can click:
    configure editor -> open/save -> Automatic cleans -> remove trailing spaces



plugins/gabble/main-options-widget.cpp
<http://git.reviewboard.kde.org/r/114034/#comment31675>

    we tend to avoid == 0
    
    account.isEmpty() returns a boolean, so here you're comparing a boolean with an integer. It will work but it's harder to read than
    
    if (! account.isEmpty()) {
    
    }


- David Edmundson


On Nov. 22, 2013, 5:31 p.m., Oskar Jauch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114034/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2013, 5:31 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-accounts-kcm
> 
> 
> Description
> -------
> 
> Fixed the Bug 326078. Now the register check box is only on add account screen of Jabber and KDETalk visible, not on edit screen.
> 
> 
> Diffs
> -----
> 
>   plugins/gabble/main-options-widget-kde-talk.cpp 4ce723f 
>   plugins/gabble/main-options-widget.cpp f2c62f5 
> 
> Diff: http://git.reviewboard.kde.org/r/114034/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Oskar Jauch
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20131122/fba0252f/attachment.html>