Review Request 125266: Authentication with certificates / Make MRU/MTU editable / Dialog fix
Lamarque Souza
lamarque at kde.org
Sun Dec 13 00:50:52 UTC 2015
-----------------------------------------------------------
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/plasma-devel/attachments/20151213/6cee0a86/attachment-0001.html>
More information about the Plasma-devel
mailing list