Remove redundant DNS field
ClosedPublic

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

Details

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.Sat, Feb 9, 3:48 AM
Restricted Application added a project: Plasma. · View Herald TranscriptSat, Feb 9, 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.Sat, Feb 9, 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.

jgrulich accepted this revision.Tue, Feb 12, 2:55 PM

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.Tue, Feb 12, 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.Wed, Feb 13, 12:47 AM
andersonbruce edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.