-
Notifications
You must be signed in to change notification settings - Fork 975
Remove keytar and gnome-keyring deps #10514
Conversation
This will make 32 bit builds on linux way easier as well. keytar is the last real dep that needs weird pkg_config options when doing the 32 bit linux build. Perfect timing. |
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.
LGTM. I assume this is possible due to the new sync library.
const passwordManagerReducer = (state, action, immutableAction) => { | ||
action = immutableAction || makeImmutable(action) | ||
switch (action.get('actionType')) { | ||
case appConstants.APP_SET_STATE: | ||
init() | ||
state = migrate(state) |
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.
I think we should also check if legacyPasswords
exists or not. If so, we should delete it to prevent our session data from getting bloated.
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.
done!
@@ -37,4 +37,3 @@ addons: | |||
packages: | |||
- xvfb | |||
- g++-4.8 | |||
- libgnome-keyring-dev |
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.
gnome-keyring is needed at runtime by muon, not sure if removing this will cause issues.
see: #10448
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.
gnome-keyring is deprecated by libsecret
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.
unless you happened to be the 3%
https://bugs.chromium.org/p/chromium/issues/detail?id=466975
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.
And I think this is the deps for keytar use not for muon which will be precompiled before browser-laptop can use it.
Give up on migrating old passwords for users who are updating from <0.15.300 to 0.21 and above. Fix #10226 Test Plan: npm run test -- --grep='passwords'
e93394f
to
01ad4d2
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.
lgtm now
Give up on migrating old passwords for users who are updating from <0.15.300 to 0.21 and above.
Fix #10226
Test Plan:
npm run test -- --grep='passwords'
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
Tests