Review Request: Import/Export Routes for VPN connections

Lamarque Vieira Souza lamarque at kde.org
Sat Mar 3 12:25:59 UTC 2012


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



libs/ui/vpnpreferences.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8961>

    setNeverDefault receives a bool parameter, not int. Please, fix this.



libs/ui/vpnpreferences.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8959>

    you should use a plain space character ' ' instead of QRegExp("\\s"). Constructing a QRegExp object is not cheap and for simple things like this one it's better use the simpler alternative.



libs/ui/vpnpreferences.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8960>

    Use the character '/' instead of the string "/". Using a character is faster than using a string.



libs/ui/vpnpreferences.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8953>

    remove trailing whitespace.



settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8954>

    join this line with the next to keep coding style.



settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8962>

    You can use VpnUiPlugin::lastErrorMessage() here. Saying that the operation did not work but not saying why is very annoying.



vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8955>

    remove trailing whitespace.



vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8957>

    remove trailing whitespace.



vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8958>

    where is the parameter for %1?



vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/104141/#comment8956>

    remove trailing whitespace.


- Lamarque Vieira Souza


On March 3, 2012, 7:45 a.m., Rajeesh K Nambiar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104141/
> -----------------------------------------------------------
> 
> (Updated March 3, 2012, 7:45 a.m.)
> 
> 
> Review request for Network Management.
> 
> 
> Description
> -------
> 
> Import/Export Routes setting for VPN connections from/to configuration files. Please review the Gateway/Metrics part in particular.
> 
> 
> Diffs
> -----
> 
>   libs/ui/vpnpreferences.cpp 9afc6f2 
>   settings/config/manageconnectionwidget.cpp caa2e06 
>   vpnplugins/vpnc/CMakeLists.txt 5af39b9 
>   vpnplugins/vpnc/vpnc.cpp 79f62ec 
> 
> Diff: http://git.reviewboard.kde.org/r/104141/diff/
> 
> 
> Testing
> -------
> 
> Tested the import/export part, Routes appear properly in the new connection. I don't have a real configuration file to test :-/
> 
> 
> Thanks,
> 
> Rajeesh K Nambiar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120303/83227a43/attachment.html>


More information about the kde-networkmanager mailing list