-
Notifications
You must be signed in to change notification settings - Fork 375
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
Update settings schema to V4 to fix a migration bug in obfuscation settings #5543
Update settings schema to V4 to fix a migration bug in obfuscation settings #5543
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.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @rablador)
ios/MullvadSettings/TunnelSettings.swift
line 31 at r1 (raw file):
case v3 = 3 /// V3 format with support for WireGuard obfuscation options with UInt16 port backing values,
I would suggest to consider more descriptive comments for these sort of schemas .I'm confused when I read them...
V4 supports V3 configuration plus back port abilities for WireGuard obfuscation
4e2ff38
to
738dda4
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.
!
Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @rablador)
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.
Dismissed @mojganii from a discussion.
Reviewable status:complete! all files reviewed, all discussions resolved
ios/MullvadSettings/TunnelSettings.swift
line 31 at r1 (raw file):
Previously, mojganii wrote…
I would suggest to consider more descriptive comments for these sort of schemas .I'm confused when I read them...
V4 supports V3 configuration plus back port abilities for WireGuard obfuscation
Reverted version changes altogether, so I'm marking this comment as resolved.
738dda4
to
31de7a1
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.
After IRL discussion it was decided to go for a smaller change without version bump to tunnel settings.
Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on @buggmagnet)
We've introduced breaking changes in the current main that necessitate a new settings schema version. We should add a migration and bump the schema version for this. This can currently be verified by installing a main version over 2023.7.
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)