-
Notifications
You must be signed in to change notification settings - Fork 30
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
BIT-2316: Fix vault timeout migration #635
Conversation
Bitwarden code coverageTotal coverage:
|
File | Coverage |
---|---|
BitwardenShared/Core/Platform/Services/StateService.swift | 97.16% |
BitwardenShared/Core/Platform/Services/Stores/AppSettingsStore.swift | 95.41% |
BitwardenShared/Core/Vault/Services/PolicyService.swift | 96.28% |
BitwardenShared/UI/Platform/Application/Views/CountdownDatePicker.swift | 0.00% |
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityProcessor.swift | 98.73% |
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityState.swift | 95.04% |
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityView.swift | 95.94% |
Powered by Slather
Generated by 🚫 Danger
❓ : Why move to an internal representation of minutes, instead of just having a migration that pulls the value from MAUI and converts it to seconds so that we can keep an internal representation of seconds? My only real concern, though, is that going to minutes potentially doesn't give us flexibility in the future for second-level precision (particularly if we wanted to do something at 90 seconds); but I'm not sure how much that flexibility matters in the long run. |
No New Or Fixed Issues Found |
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.
This looks great!
Oh, sorry I missed this! I was mainly just thinking that it made sense to remain consistent with Maui and Android. I don't feel that strongly for either approach though. Do you prefer the migration? |
I'm fine either way, realistically, and if Android's also going the minutes route, might as well remain consistent. Looks good to me! |
🎟️ Tracking
BIT-2316
📔 Objective
We were storing the vault timeout setting in seconds, but Xamarin/Maui was storing it in minutes. This updates our handling to expect it in minutes.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes