Review Request 125543: OpenVPN improvements

Discussion:

Jan Grulich

2015-10-07 08:17:34 UTC

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

Review request for Network Management and Lamarque Souza.

Repository: plasma-nm

Description
-------

While adding tooltips to our editor for OpenVPN properties I noticed that we are missing options for few properties which are already implemented in nm-connection-editor and also that we have different names for some options. This patch adds those missing properties and changes some labels so they match NM implementation. I also fixed some coding style issues and used QLatin1String where it was possible.

Diffs
-----

vpn/openvpn/openvpnadvancedwidget.cpp fe63958
vpn/openvpn/openvpnadvanced.ui 687d7d1
vpn/openvpn/openvpn.ui b68e27e
vpn/openvpn/nm-openvpn-service.h 39e2251

Diff: https://git.reviewboard.kde.org/r/125543/diff/

Testing
-------

Thanks,

Jan Grulich

Lamarque Souza

2015-10-07 11:42:52 UTC

Permalink

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

vpn/openvpn/openvpnadvanced.ui (line 226)
<https://git.reviewboard.kde.org/r/125543/#comment59559>

Since this is a tooltip I prefer not to use abbreviations (like MSS) here, use "maximum segment size" instead. TCP is ok because those three letters are well known today, more than their meaning (transmission control protocol).

vpn/openvpn/openvpnadvancedwidget.cpp (line 193)
<https://git.reviewboard.kde.org/r/125543/#comment59560>

const

vpn/openvpn/openvpnadvancedwidget.cpp (line 247)
<https://git.reviewboard.kde.org/r/125543/#comment59561>

You can use QLatin1String around "http" here. I think even QStringLiteral works here too.

vpn/openvpn/openvpnadvancedwidget.cpp (line 249)
<https://git.reviewboard.kde.org/r/125543/#comment59562>

Same here.

vpn/openvpn/openvpnadvancedwidget.cpp (line 261)
<https://git.reviewboard.kde.org/r/125543/#comment59563>

And here.

- Lamarque Souza

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/125543/
-----------------------------------------------------------
(Updated Oct. 7, 2015, 8:17 a.m.)
Review request for Network Management and Lamarque Souza.
Repository: plasma-nm
Description
-------
While adding tooltips to our editor for OpenVPN properties I noticed that we are missing options for few properties which are already implemented in nm-connection-editor and also that we have different names for some options. This patch adds those missing properties and changes some labels so they match NM implementation. I also fixed some coding style issues and used QLatin1String where it was possible.
Diffs
-----
vpn/openvpn/openvpnadvancedwidget.cpp fe63958
vpn/openvpn/openvpnadvanced.ui 687d7d1
vpn/openvpn/openvpn.ui b68e27e
vpn/openvpn/nm-openvpn-service.h 39e2251
Diff: https://git.reviewboard.kde.org/r/125543/diff/
Testing
-------
Thanks,
Jan Grulich

Jan Grulich

2015-10-07 11:52:17 UTC

Permalink

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

(Updated ÅÃj. 7, 2015, 11:52 dop.)

Review request for Network Management and Lamarque Souza.

Changes
-------

Fixed mentioned issues.

Repository: plasma-nm

Description
-------

While adding tooltips to our editor for OpenVPN properties I noticed that we are missing options for few properties which are already implemented in nm-connection-editor and also that we have different names for some options. This patch adds those missing properties and changes some labels so they match NM implementation. I also fixed some coding style issues and used QLatin1String where it was possible.

Diffs (updated)
-----

vpn/openvpn/nm-openvpn-service.h 39e2251
vpn/openvpn/openvpn.ui b68e27e
vpn/openvpn/openvpnadvanced.ui 687d7d1
vpn/openvpn/openvpnadvancedwidget.cpp fe63958

Diff: https://git.reviewboard.kde.org/r/125543/diff/

Testing
-------

Thanks,

Jan Grulich

Lamarque Souza

2015-10-07 11:54:23 UTC

Permalink

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/125543/
-----------------------------------------------------------
(Updated Oct. 7, 2015, 11:52 a.m.)
Review request for Network Management and Lamarque Souza.
Repository: plasma-nm
Description
-------
While adding tooltips to our editor for OpenVPN properties I noticed that we are missing options for few properties which are already implemented in nm-connection-editor and also that we have different names for some options. This patch adds those missing properties and changes some labels so they match NM implementation. I also fixed some coding style issues and used QLatin1String where it was possible.
Diffs
-----
vpn/openvpn/nm-openvpn-service.h 39e2251
vpn/openvpn/openvpn.ui b68e27e
vpn/openvpn/openvpnadvanced.ui 687d7d1
vpn/openvpn/openvpnadvancedwidget.cpp fe63958
Diff: https://git.reviewboard.kde.org/r/125543/diff/
Testing
-------
Thanks,
Jan Grulich

Jan Grulich

2015-10-07 12:06:06 UTC

Permalink

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

(Updated Oct. 7, 2015, 12:06 p.m.)

Status
------

This change has been marked as submitted.

Review request for Network Management and Lamarque Souza.

Changes
-------

Submitted with commit 1a348c01d421f6ce5479e66ba7f72a58646bc2a3 by Jan Grulich to branch master.

Repository: plasma-nm

Description
-------

While adding tooltips to our editor for OpenVPN properties I noticed that we are missing options for few properties which are already implemented in nm-connection-editor and also that we have different names for some options. This patch adds those missing properties and changes some labels so they match NM implementation. I also fixed some coding style issues and used QLatin1String where it was possible.

Diffs
-----

vpn/openvpn/nm-openvpn-service.h 39e2251
vpn/openvpn/openvpn.ui b68e27e
vpn/openvpn/openvpnadvanced.ui 687d7d1
vpn/openvpn/openvpnadvancedwidget.cpp fe63958

Diff: https://git.reviewboard.kde.org/r/125543/diff/

Testing
-------

Thanks,

Jan Grulich