-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PIA-1291: Make wireguard the default option #28
PIA-1291: Make wireguard the default option #28
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kp-juan-docal , it looks good to me. I just added some comments about how we can only set this up for iOS
@@ -225,7 +225,7 @@ extension Client { | |||
enablesServerPings = false | |||
minPingInterval = 120000 | |||
|
|||
availableVPNProfiles = [IKEv2Profile()] | |||
availableVPNProfiles = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we apply this update only when the platform is not tvOS?
@@ -481,7 +495,7 @@ extension Client.Preferences { | |||
useWiFiProtection = true | |||
trustCellularData = false | |||
nmtMigrationSuccess = false | |||
vpnType = IKEv2Profile.vpnType | |||
vpnType = PIAWGTunnelProfile.vpnType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the other comments: for tvOS we still want IKEv2 to be the default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to also have all references to PIAWGTunnelProfile inside the iOS condition since this class is not available on tvOS otherwise it won't compile on tvOS
ca45534
to
18e340f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @kp-juan-docal! Just one comment to address in order to have it compile for tvOS as well
18e340f
to
b751e20
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kp-juan-docal , LGTM ✅
Summary
It introduces the changes to make wireguard the default protocol on new installations and the logic to allow us to migrate to wireguard those on ikev2 with default settings.
Sanity Testers
Tests were performed with the application changes introduced here