-
Notifications
You must be signed in to change notification settings - Fork 155
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
Enable identity pinning violation notifications unconditionally #3745
Enable identity pinning violation notifications unconditionally #3745
Conversation
Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:
|
(Remove the feature flag we added when this feature seemed unstable.)
10b5e0e
to
e362cad
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3745 +/- ##
===========================================
- Coverage 82.93% 82.92% -0.02%
===========================================
Files 1770 1770
Lines 44465 44453 -12
Branches 5222 5221 -1
===========================================
- Hits 36878 36863 -15
- Misses 5752 5753 +1
- Partials 1835 1837 +2 ☔ View full report in Codecov by Sentry. |
…fs-unconditionally
Test failure looks unrelated, unless I'm missing something? |
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! We have a flaky test, I have re-run the test, it should be fine.
@bmarty would you like me to rebase before merging? |
@andybalaam no, you can merge anytime! |
…fs-unconditionally
Content
Enable identity pinning violation notifications unconditionally (remove the feature flag we added when this feature seemed unstable).
Motivation and context
(Internal issue). Part of the Trust and Decorations work.
In #3648 we disabled identity change notifications based on a default-off feature flag, because we found some bugs in the feature. The bugs are fixed and we think it's time to enable the feature unconditionally.
Tests
On a local Synapse, I connected 2 users and changed the identity of one. I verified that, despite not setting any labs flags, the "identity changed" banner appeared.
Tested devices
Checklist
UI change has been tested on both light and dark themes- no UI changeAccessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility- no UI changePull request includes screenshots or videos if containing UI changes- no UI changesPull request includes a sign off- I am exempt because I am an Element employee