Differential D18867

Remove redundant DNS field
ClosedPublic
Actions

Authored by andersonbruce on Feb 9 2019, 3:48 AM.

Details

Reviewers

jgrulich

Commits

R116:29b6d0ee5900: Remove redundant DNS field

Summary

BUG: 403546

A DNS field was included on the VPN tab in the initial release to match the
interface of the underlying NetworkManager plugin. This change removes this field and instead
requires the user to use the standard DNS field on the IPv4 and/or IPv6 tabs to enter DNS
servers.
The import function was changed to insert any DNS servers specified in the incoming config
file into the new locations and the export function was changed to output DNS servers from
the IPv4 and IPv6 tabs if they are present but for compatibility with connections created
with the previous version or with the basic NetworkManager plugin, if the IPv4 and IPv6 tabs
do not contain DNS and an (undisplayed) entry on the VPN tab is present it will export from
there. This allows a user to update a connection to the new format by simply exporting it,
deleting it, and re-importing it.

Diff Detail

Repository

R116 Plasma Network Management Applet

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

andersonbruce created this revision.Feb 9 2019, 3:48 AM

Restricted Application added a project: Plasma. · View Herald TranscriptFeb 9 2019, 3:48 AM

Restricted Application added 1 blocking reviewer(s): jgrulich. · View Herald Transcript

Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

andersonbruce requested review of this revision.Feb 9 2019, 3:48 AM

Harbormaster completed remote builds in B8056: Diff 51235.Feb 9 2019, 3:48 AM

Any connection created by the previous version or the basic NetworkManager plugin should continue to work even though the DNS server list will not be viewable in the new interface.
I did not provide an automatic conversion from a previously created connection to the new storage format. Neither did I create a warning (popup?) if an old style config is displayed by the editor. If there is a consensus to add either of these items, I can do that.

Migrating sounds like a good idea. I don't think popups are necessary for this.

I wonder whether this should also go to Plasma 5.15 branch, it's an improvement and doesn't introduce new strings so should be ok to be backported.

This revision is now accepted and ready to land.Feb 12 2019, 2:55 PM

Does this fix 403546? If so, then the Summary section should have BUG: 403546 in it so the bug automatically gets closed. Also regardless, You should remove "per bug 403546" from the title. See https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch

andersonbruce retitled this revision from Remove redundant DNS field per bug 403546 to Remove redundant DNS field.Feb 13 2019, 12:47 AM

andersonbruce edited the summary of this revision. (Show Details)

Closed by commit R116:29b6d0ee5900: Remove redundant DNS field (authored by andersonbruce, committed by jgrulich). · Explain WhyFeb 13 2019, 7:10 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			libs/editor/connectioneditorbase.cpp (3 lines)
M			vpn/wireguard/wireguard.cpp (75 lines)
M			vpn/wireguard/wireguard.ui (15 lines)
M			vpn/wireguard/wireguardwidget.h (1 line)
M			vpn/wireguard/wireguardwidget.cpp (23 lines)