Review Request 125266: Authentication with certificates / Make MRU/MTU editable / Dialog fix

[Lamarque Souza]

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



vpn/l2tp/l2tppppwidget.cpp (line 107)
<https://git.reviewboard.kde.org/r/125266/#comment61154>

    Remove extra spaces.



vpn/l2tp/l2tppppwidget.cpp (line 108)
<https://git.reviewboard.kde.org/r/125266/#comment61155>

    Why compare a QString to NULL? You should use:
    
    if (dataMap.contains(QLatin1String(NM_L2TP_KEY_MTU))) {
    ...
    }
    
    Code style: always use brackets with if, even if there is just one line. Also add space after the if.



vpn/l2tp/l2tppppwidget.cpp (line 110)
<https://git.reviewboard.kde.org/r/125266/#comment61156>

    Remove extra space.



vpn/l2tp/l2tppppwidget.cpp (line 111)
<https://git.reviewboard.kde.org/r/125266/#comment61157>

    if (dataMap.contains(QLatin1String(NM_L2TP_KEY_MRU))) {
    ...
    }



vpn/l2tp/l2tppppwidget.cpp (line 189)
<https://git.reviewboard.kde.org/r/125266/#comment61158>

    Remove space.



vpn/l2tp/l2tppppwidget.cpp (line 190)
<https://git.reviewboard.kde.org/r/125266/#comment61161>

    Code style:
    
    if (...) {
    }



vpn/l2tp/l2tppppwidget.cpp (line 192)
<https://git.reviewboard.kde.org/r/125266/#comment61159>

    Remove space.



vpn/l2tp/l2tppppwidget.cpp (line 193)
<https://git.reviewboard.kde.org/r/125266/#comment61162>

    Code style:
    
    if (...) {
    }



vpn/l2tp/l2tppppwidget.cpp (line 195)
<https://git.reviewboard.kde.org/r/125266/#comment61160>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 91)
<https://git.reviewboard.kde.org/r/125266/#comment61163>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 92)
<https://git.reviewboard.kde.org/r/125266/#comment61168>

    Use if (data.contains()) to check if the data exists before retrieving it with data.value(). That way you avoid allocating a temporary QString when not necessary.



vpn/l2tp/l2tpwidget.cpp (line 96)
<https://git.reviewboard.kde.org/r/125266/#comment61164>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 97)
<https://git.reviewboard.kde.org/r/125266/#comment61169>

    Use data.contains()



vpn/l2tp/l2tpwidget.cpp (line 101)
<https://git.reviewboard.kde.org/r/125266/#comment61165>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 102)
<https://git.reviewboard.kde.org/r/125266/#comment61170>

    Use data.contains()



vpn/l2tp/l2tpwidget.cpp (line 106)
<https://git.reviewboard.kde.org/r/125266/#comment61166>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 107)
<https://git.reviewboard.kde.org/r/125266/#comment61171>

    Use QStringLiteral("yes") instead of just "yes". Add brackets after the parentesis and space after if.



vpn/l2tp/l2tpwidget.cpp (line 135)
<https://git.reviewboard.kde.org/r/125266/#comment61172>

    This should be a QScopedPointer instead of QPointer or you will have a memory leak here.
    
    Do not use NULL, use Q_NULLPTR instead.



vpn/l2tp/l2tpwidget.cpp (line 138)
<https://git.reviewboard.kde.org/r/125266/#comment61167>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 143)
<https://git.reviewboard.kde.org/r/125266/#comment61173>

    QScopedPointer instead of QPointer.



vpn/l2tp/l2tpwidget.cpp (line 146)
<https://git.reviewboard.kde.org/r/125266/#comment61174>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 152)
<https://git.reviewboard.kde.org/r/125266/#comment61175>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 155)
<https://git.reviewboard.kde.org/r/125266/#comment61176>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 156)
<https://git.reviewboard.kde.org/r/125266/#comment61178>

    Code style:
    
    if (...) {
    }



vpn/l2tp/l2tpwidget.cpp (line 162)
<https://git.reviewboard.kde.org/r/125266/#comment61177>

    Remove space.



vpn/l2tp/l2tpwidget.cpp (line 265)
<https://git.reviewboard.kde.org/r/125266/#comment61179>

    Code style:
    
    if (...) {
    } else {
    }


- Lamarque Souza


On Dec. 12, 2015, 11:11 p.m., René Fürst wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125266/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2015, 11:11 p.m.)
> 
> 
> Review request for Network Management and Plasma.
> 
> 
> Repository: plasma-nm
> 
> 
> Description
> -------
> 
> Authentication with certificates / Make MRU/MTU editable / Dialog fix
> 
> 
> Diffs
> -----
> 
>   vpn/l2tp/l2tpwidget.cpp b278228 
>   vpn/l2tp/nm-l2tp-service.h ac2ecc9 
>   vpn/l2tp/l2tpwidget.h 1e4c383 
>   vpn/l2tp/l2tppppwidget.cpp ffe2c2b 
>   vpn/l2tp/l2tp.ui 22b9f73 
>   vpn/l2tp/l2tpppp.ui 7e4fea3 
> 
> Diff: https://git.reviewboard.kde.org/r/125266/diff/
> 
> 
> Testing
> -------
> 
> This patch contains 3 things:
> 1) Add authentication with certificates
> 2) Make MRU/MTU editable
> 3) Fix an issue where PPP/Advanced settings were lost when the dialogs were not opened
> 
> The base for 1) and 2) was added to NetworkManager-l2tp here https://github.com/frenetic1/NetworkManager-l2tp/commit/8103cf09e2cda19d701a48331eba069ff4c8e82c
> 
> 
> Thanks,
> 
> René Fürst
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20151213/6cee0a86/attachment-0001.html>