-----------------------------------------------------------
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
Post by René Fürst-----------------------------------------------------------
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
-------
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