Review Request: [RFC] Import VPN connection

[Lamarque Vieira Souza]

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



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

    Shouldn't you also emit a notification or a popup warning the user that the import failed?



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

    Please, remove extra line.



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

    If this button is visible you should also emit a notification or a popup warning that this has not been implemented yet. Or just hide the export button.



vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2944>

    Notification or popup informing about failed execution.



vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2945>

    Please remove space at line start. Also, you do not insert the password into the connection map if the executable failed to run. I guess the VPN connection is not going to work in that case, right? If so I think you should abort the import process. Think of this function as a (SQL) transation: or everything works or the entire import process is aborted.



vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2946>

    Same here: remove extra space at line start and transation (or everything works or the entire import process is aborted).
    
    We already have some sutle bugs in VPN code very difficult to debug/fix. If we can prevent some more to show up in new code the better.


- Lamarque Vieira


On May 24, 2011, 2:09 p.m., Rajeesh K Nambiar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101419/
> -----------------------------------------------------------
> 
> (Updated May 24, 2011, 2:09 p.m.)
> 
> 
> Review request for Network Management.
> 
> 
> Summary
> -------
> 
> First stab at VPN connection import/export functionality. Currently implemented just VPNC Import support. Please review, especially the VpncUiPluginPrivate part which tries to abstract away cisco password decrypt function. If the general approach looks good, I'll proceed with this and try to extend for other VPN methods, as well as export function.
> 
> 
> This addresses bug 146159.
>     http://bugs.kde.org/show_bug.cgi?id=146159
> 
> 
> Diffs
> -----
> 
>   libs/ui/vpnpreferences.cpp 843636c 
>   libs/ui/vpnuiplugin.h 7a13027 
>   settings/config/CMakeLists.txt 268c23b 
>   settings/config/addeditdeletebuttonset.h f7abef7 
>   settings/config/addeditdeletebuttonset.cpp 4f3f97a 
>   settings/config/manageconnectionwidget.h 51f60a0 
>   settings/config/manageconnectionwidget.cpp 46910db 
>   vpnplugins/novellvpn/novellvpn.h 9e026e2 
>   vpnplugins/novellvpn/novellvpn.cpp 848b527 
>   vpnplugins/openvpn/openvpn.h a06b88e 
>   vpnplugins/openvpn/openvpn.cpp 60376ed 
>   vpnplugins/pptp/pptp.h 66ea79a 
>   vpnplugins/pptp/pptp.cpp c311f9f 
>   vpnplugins/strongswan/strongswan.h fcd5bde 
>   vpnplugins/strongswan/strongswan.cpp 5bffc2b 
>   vpnplugins/vpnc/vpnc.h aec2136 
>   vpnplugins/vpnc/vpnc.cpp deb9108 
> 
> Diff: http://git.reviewboard.kde.org/r/101419/diff
> 
> 
> Testing
> -------
> 
> Tested against latest git snapshot, with KDE SC 4.6.3
> 
> 
> Thanks,
> 
> Rajeesh
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110524/b6efe20a/attachment-0001.htm