-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-14445] TS strict for Key Management Biometrics #13039
base: main
Are you sure you want to change the base?
Conversation
Great job, no security vulnerabilities found in this Pull Request |
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.
CL type change looks good; ty!
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.
Changes look good from my side for the KM owned biometrics code; I have added some minor suggestions for improvement.
6d0f006
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.
Changes to KM code LGTM!
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.
Platform changes look great overall, happy to have these annotated. Just a couple questions about a couple hopefully unnecessary changes. So that we can avoid everyone having to change their own reference of GlobalState<MyType>
to GlobalState<MyType | null>
.
8aa0f72
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-14445
📔 Objective
Due to change on null expectations in state (platform), a lot of code all around had to change to reflect that too, that are already on strict mode too.
Enabled TypeScript strict mode for Key Management biometrics owned code.
Plus small refactoring around secure channel in
apps/browser/src/background/nativeMessaging.background.ts
to make it easier.📸 Screenshots
⏰ 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