Review Request: rakia plugin for telepathy-accounts-kcm

[George Kiagiadakis]

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



rakia/kcmtelepathyaccounts_plugin_rakia.desktop
<http://git.reviewboard.kde.org/r/100444/#comment1203>

    Your code seems to be LGPL actually, but it doesn't matter much



rakia/rakia-account-ui-plugin.h
<http://git.reviewboard.kde.org/r/100444/#comment1207>

    Too long line, please line-wrap.



rakia/rakia-account-ui-plugin.cpp
<http://git.reviewboard.kde.org/r/100444/#comment1208>

    Yet another useless private class...



rakia/rakia-account-ui-plugin.cpp
<http://git.reviewboard.kde.org/r/100444/#comment1209>

    Why does this line have a \ at the end? Does that compile?



rakia/rakia-account-ui.cpp
<http://git.reviewboard.kde.org/r/100444/#comment1204>

    Can you please re-wrap all the lines in this function so that the comment is above the line of code that it comments? The lines are too long and it becomes really hard to read when the comment is line-wrapped below in the editor.



rakia/rakia-account-ui.cpp
<http://git.reviewboard.kde.org/r/100444/#comment1205>

    This comment is also hard to read and ugly. Do we need it? If you leave it there, line wrap appropriately and add empty separator lines (i.e. make it readable) please.



rakia/rakia-account-ui.cpp
<http://git.reviewboard.kde.org/r/100444/#comment1206>

    i18n("Advanced Options")



rakia/rakia-advanced-options-widget.cpp
<http://git.reviewboard.kde.org/r/100444/#comment1210>

    These lines are also very long. Try to keep them below 100 columns.



rakia/rakia-advanced-options-widget.cpp
<http://git.reviewboard.kde.org/r/100444/#comment1211>

    Remove spaces from the signal and slot signatures. They add runtime overhead for removing them.


- George


On Feb. 13, 2011, 5:35 p.m., Florian Reinhard wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100444/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2011, 5:35 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> the following telepathy-sofiasip parameters are implemented:
> account
> password
> alias
> 
> auth-user
> registrar
> 
> proxy-host
> port
> transport
> 
> discover-stun
> stun-server
> stun-port
> 
> discover-binding
> loose-routing
> keepalive-mechanism
> keepalive-interval
> 
> 
> these are missing atm:
> immutable-streams
> local-ip-address
> local-port
> extra-auth-user
> extra-auth-password
> 
> 
> Diffs
> -----
> 
>   rakia/rakia-account-ui.h PRE-CREATION 
>   rakia/kcmtelepathyaccounts_plugin_rakia.desktop PRE-CREATION 
>   rakia/rakia-account-ui-plugin.h PRE-CREATION 
>   rakia/rakia-account-ui-plugin.cpp PRE-CREATION 
>   rakia/Messages.sh PRE-CREATION 
>   CMakeLists.txt 9aec0ecf2243e302d3cd01f1c4bd246f6e248305 
>   rakia/CMakeLists.txt PRE-CREATION 
>   rakia/rakia-account-ui.cpp PRE-CREATION 
>   rakia/rakia-advanced-options-widget.h PRE-CREATION 
>   rakia/rakia-advanced-options-widget.cpp PRE-CREATION 
>   rakia/rakia-advanced-options-widget.ui PRE-CREATION 
>   rakia/rakia-main-options-widget.h PRE-CREATION 
>   rakia/rakia-main-options-widget.cpp PRE-CREATION 
>   rakia/rakia-main-options-widget.ui PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100444/diff
> 
> 
> Testing
> -------
> 
> add new account
> edit existing account
> connect
> 
> 
> Screenshots
> -----------
> 
> Main Options and Advanced Options
>   http://git.reviewboard.kde.org/r/100444/s/55/
> as implemented in diff r3
>   http://git.reviewboard.kde.org/r/100444/s/76/
> 
> 
> Thanks,
> 
> Florian
> 
>

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