Review Request: Created haze AIM plugin

[George Kiagiadakis]

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


The code is ok. Mostly whitespace problems noticed.

The commits in the branch are not ok, though. You seem to have merged master into your branch, which should be avoided. Instead, when you wish to update your branch to include the latest master, use git rebase (git rebase master your-branch).

To fix this now, you need to create a new branch that is based on master and cherry-pick the two relevant commits there:
$ git checkout -b aol-plugin master
$ git cherry-pick e8210fde850d5e19319fffed55b5f3fbebec5341
$ git cherry-pick a7ff93873b9ea44a93f17e60f9f8a8aef73b9897

Then you can continue working on this new aol-plugin branch.


haze/aim-main-options-widget.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1284>

    Trailing whitespace.



haze/aim-main-options-widget.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1291>

    1) If this is temporary debugging code, it shouldn't be in the commit (you can keep it on git stash if you wish)
    
    2) If this is here to stay, it needs:
    * const Tp::ProtocolParameter & param
    * line-wrap the kDebug() statement, as it is too long.



haze/haze-account-ui-plugin.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1286>

    I think this comment doesn't make much sense to be kept and updated. It will end up being a huge comment listing all the protocols that are listed below as well (and the code is very clear to read)



haze/haze-account-ui-plugin.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1285>

    According to the kdelibs coding style, if statements with only one statement inside them should also have {} braces. For example:
    
    if (foo) {
       stmt;
    }
    



haze/haze-account-ui-plugin.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1287>

    Trailing whitespace.



haze/haze-account-ui-plugin.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1288>

    Trailing whitespace.



haze/haze-account-ui-plugin.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1289>

    Trailing whitespace.



haze/yahoo-main-options-widget.cpp
<http://git.reviewboard.kde.org/r/100689/#comment1290>

    This is probably, once more, a git problem. It shouldn't be all in the diff. The only part that is part of the diff is the change in lines 28-29, which also adds trailing whitespace...


- George


On Feb. 20, 2011, 6:10 a.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100689/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2011, 6:10 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> I created a AIM plugin, with basic and advanced options in one go this time.
> 
> It's in my personal clone
> http://quickgit.kde.org/?p=clones/telepathy-accounts-kcm-plugins/fernando/main.git&a=shortlog&h=refs/heads/aol-plugin-build
> 
> (branch called 'aol-plugin-build')
> 
> 
> Diffs
> -----
> 
>   haze/CMakeLists.txt 67d43a1 
>   haze/aim-main-options-widget.h PRE-CREATION 
>   haze/aim-main-options-widget.cpp PRE-CREATION 
>   haze/aim-main-options-widget.ui PRE-CREATION 
>   haze/aim-server-settings-widget.h PRE-CREATION 
>   haze/aim-server-settings-widget.cpp PRE-CREATION 
>   haze/aim-server-settings-widget.ui PRE-CREATION 
>   haze/haze-account-ui-plugin.cpp 415ebaa 
>   haze/haze-aim-account.h PRE-CREATION 
>   haze/haze-aim-account.cpp PRE-CREATION 
>   haze/yahoo-main-options-widget.cpp 8beec3c 
> 
> Diff: http://git.reviewboard.kde.org/r/100689/diff
> 
> 
> Testing
> -------
> 
> Edited options and made sure they were reflected in Empathy.
> 
> 
> Thanks,
> 
> Lasath
> 
>

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