jgrulich added a subscriber: ngraham.
jgrulich added a comment.
Can we maybe have some assistence from anyone from VDG? @ngraham maybe?
INLINE COMMENTS
> CMakeLists.txt:11
>
> +set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g")
> +
Please remove.
> kcm.cpp:264
> NetworkManager::ConnectionSettings::ConnectionType type =
> static_cast<NetworkManager::ConnectionSettings::ConnectionType>(connectionType);
> -
> +qCWarning(PLASMA_NM) << "onRequestCreateConnection type: " << type;
> if (type == NetworkManager::ConnectionSettings::Vpn && vpnType ==
> "imported") {
Please remove.
> connectionicon.cpp:403
> }
> + } else if (type == 29) { // TODO change to WireGuard enum
> value once it is added
> + // WireGuard is a VPN but is not implemented
Do you want me to add this to NMQT?
> connectioneditorbase.cpp:151
> }
> -
> +#if 1
> +qCWarning(PLASMA_NM) << "ConnectionEditorBase: Before toMap";
Please remove
> connectioneditorbase.cpp:198
>
> + qCWarning(PLASMA_NM) << "ConnectionEditorBase:" << m_connection->id();
> +
Please remove.
> connectioneditorbase.cpp:264
> if (!vpnSetting) {
> - qCWarning(PLASMA_NM) << "Missing VPN setting!";
> + qCWarning(PLASMA_NM) << "Missing VPN setting 2!";
> } else {
Why the "2" at the end?
> connectioneditorbase.cpp:458
> for (const QString &key : secrets.keys()) {
> + qCWarning(PLASMA_NM) << "replyFinished key: " << key << "
> settingName: " << settingName;
> if (key == settingName) {
Please remove.
> connectioneditorbase.cpp:465
> const QString type = widget->type();
> +qCWarning(PLASMA_NM) << "type: " << type << " settingName: " <<
> settingName;
> if (type == settingName ||
Please remove.
> wireguardinterfacewidget.cpp:114
> +WireGuardInterfaceWidget::WireGuardInterfaceWidget(const
> NetworkManager::Setting::Ptr &setting, QWidget* parent, Qt::WindowFlags f):
> + SettingWidget(setting, parent, f),
> + d(new Private)
Coding style.
Should be:
WireGuardInterfaceWidget::WireGuardInterfaceWidget(const
NetworkManager::Setting::Ptr &setting, QWidget* parent, Qt::WindowFlags f)
: SettingWidget(setting, parent, f)
, d(new Private)
> wireguardinterfacewidget.cpp:182
> +
> + if (0 != wireGuardSetting->listenPort())
> +
> d->ui.listenPortLineEdit->setText(QString::number(wireGuardSetting->listenPort()));
I would preffer:
if (wireGuardSetting->listenPort() != 0)
It's more readable. Same for code below.
> wireguardinterfacewidget.cpp:250
> +{
> +
> + NetworkManager::WireGuardSetting wgSetting;
Remove empty line
> wireguardinterfacewidget.cpp:424
> +
> + else if (keyValue[0] == PNM_WG_CONF_TAG_PEER) {
> + // Check to make sure the previous PEER section has
Coding style. The "else if" should be on the same line as the "}" bracket.
> wireguardinterfacewidget.cpp:482
> + // Listen Port
> + else if (key == PNM_WG_CONF_TAG_LISTEN_PORT) {
> + uint val = keyValue[1].toUInt();
Same here and below.
> wireguardinterfacewidget.cpp:568
> + else if (currentState == PEER_SECTION)
> + {
> + // Public Key
The "{" bracket should be on the same line as the "else if". Same below.
> wireguardinterfacewidget.cpp:633
> + peers.append(*currentPeer);
> +#endif
> +#if 1
Please remove
> wireguardpeerswidget.cpp:82
> +
> +WireGuardPeersWidget::WireGuardPeersWidget(const NMVariantMapList& peerData,
> QWidget* parent, Qt::WindowFlags f):
> + QDialog(parent, f),
Coding style.
REPOSITORY
R116 Plasma Network Management Applet
REVISION DETAIL
https://phabricator.kde.org/D20930
To: andersonbruce, jgrulich
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
