Review Request 124279: Asynchronous loading of secrets in the connection editor

Lamarque Souza lamarque at kde.org
Tue Jul 7 14:15:47 UTC 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124279/#review82170
-----------------------------------------------------------



libs/editor/connectiondetaileditor.cpp (line 189)
<https://git.reviewboard.kde.org/r/124279/#comment56557>

    Why not use camel case in this property's name?



libs/editor/connectiondetaileditor.cpp (line 419)
<https://git.reviewboard.kde.org/r/124279/#comment56565>

    Nitpick: two different code styles for pointers here.



libs/editor/widgets/settingwidget.h (line 43)
<https://git.reviewboard.kde.org/r/124279/#comment56559>

    From API point of view since now there is a loadScrets() method it should be the only method used to load secrets all over the place. In this patch you kept loadConfig() loading secrets too, which is code duplication.
    
    Another alternative is keeping the code as it is now and just add a QProgressDialog [1] to indicate to the user that secrets are being loaded. QProgressDialog only shows up when it calculates (internally) that it will be visible for more than 4 seconds (default value, it is adjustable). This way the dialog appears only when loading secrets is taking too much time. We can make dialog modal to prevent user from saving the connection configuration.
    
    [1] http://doc.qt.io/qt-5/qprogressdialog.html


- Lamarque Souza


On July 7, 2015, 8:15 a.m., Jan Grulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124279/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 8:15 a.m.)
> 
> 
> Review request for Network Management and Lamarque Souza.
> 
> 
> Bugs: 349002
>     http://bugs.kde.org/show_bug.cgi?id=349002
> 
> 
> Repository: plasma-nm
> 
> 
> Description
> -------
> 
> Previously the UI of the editor was completely loaded only when we got respond from NM with or without secrets. Problem is that in some cases, eg. when our kded module is not loaded, the request will fail and we need to wait until the request times out, which leads to a problem described in BUG 349002. I changed this behaviour to make the UI load immediately and load secrets additionaly in case we get respond from NM, otherwise the UI will be loaded without secrets. The only problem I can think of is when secrets won't be loaded and the user will save its connection without secrets, but in most cases it shouldn't let him to press the OK button because of validation.
> 
> 
> Diffs
> -----
> 
>   editor/connectioneditor.h 1776a36 
>   libs/editor/connectiondetaileditor.cpp f84a197 
>   libs/editor/settings/cdmawidget.h 99b6b43 
>   libs/editor/settings/cdmawidget.cpp 566670c 
>   libs/editor/settings/gsmwidget.h ce11371 
>   libs/editor/settings/gsmwidget.cpp fff73d8 
>   libs/editor/settings/pppoewidget.h 45df517 
>   libs/editor/settings/pppoewidget.cpp 42e117e 
>   libs/editor/settings/security802-1x.h 6f79fc7 
>   libs/editor/settings/security802-1x.cpp 0ecc333 
>   libs/editor/settings/wifisecurity.h fd2e006 
>   libs/editor/settings/wifisecurity.cpp 71d700c 
>   libs/editor/settings/wiredsecurity.h cfe7f78 
>   libs/editor/settings/wiredsecurity.cpp 616aa70 
>   libs/editor/widgets/settingwidget.h 8d07b73 
>   libs/editor/widgets/settingwidget.cpp 146113d 
>   vpn/l2tp/l2tpwidget.h eed2b24 
>   vpn/l2tp/l2tpwidget.cpp fa6118b 
>   vpn/openswan/openswanwidget.h ce5a04f 
>   vpn/openswan/openswanwidget.cpp ca98e6d 
>   vpn/openvpn/openvpnwidget.h d7afcdb 
>   vpn/openvpn/openvpnwidget.cpp 18b0b70 
>   vpn/pptp/pptpwidget.h 1645a92 
>   vpn/pptp/pptpwidget.cpp a540c81 
>   vpn/ssh/sshwidget.h f05caf8 
>   vpn/ssh/sshwidget.cpp 5d077c7 
>   vpn/sstp/sstpwidget.h 1c60252 
>   vpn/sstp/sstpwidget.cpp 67c54ca 
>   vpn/strongswan/strongswanwidget.h 8101bb7 
>   vpn/strongswan/strongswanwidget.cpp 88a2ede 
>   vpn/vpnc/vpncwidget.h 69beb97 
>   vpn/vpnc/vpncwidget.cpp 4d5330d 
> 
> Diff: https://git.reviewboard.kde.org/r/124279/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20150707/0f1a3ffd/attachment.html>


More information about the kde-networkmanager mailing list