Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Changes persistence level to 'enterprise' on Windows #191

Merged
merged 1 commit into from
Jul 30, 2019

Conversation

jakobvogel
Copy link
Contributor

Restores #123. Closes #168.

@bkostjens
Copy link

bkostjens commented Jul 23, 2019

Any update on this PR?

@shiftkey
Copy link
Contributor

@bkostjens it's on my list to review this week - I want to do some verification of this behaviour to better understand the migration story (if needed) myself...

@bkostjens
Copy link

@bkostjens it's on my list to review this week - I want to do some verification of this behaviour to better understand the migration story (if needed) myself...

Ah perfect! Thanks for your quick response.

Copy link
Contributor

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested out various upgrade/downgrade scenarios here and I'm happy this is a safe change. But to make this really clear to others I plan to ship this as part of a v5 beta to gather feedback later this week.

Thanks again for working through this with me @jakobvogel!

@shiftkey shiftkey changed the title Changes Windows persistence level to 'enterprise' Changes Windows persistence level to 'enterprise' on Windows Jul 30, 2019
@shiftkey shiftkey changed the title Changes Windows persistence level to 'enterprise' on Windows Changes persistence level to 'enterprise' on Windows Jul 30, 2019
@shiftkey shiftkey merged commit 0b0c896 into atom:master Jul 30, 2019
@jakobvogel jakobvogel deleted the enterprise-persistence branch July 30, 2019 19:29
@jakobvogel
Copy link
Contributor Author

Thank you @shiftkey! 👍

@shiftkey
Copy link
Contributor

@jakobvogel https://github.com/atom/node-keytar/releases/tag/v5.0.0-beta.0 is available to test out and verify you're happy with the change.

@jakobvogel
Copy link
Contributor Author

@jakobvogel https://github.com/atom/node-keytar/releases/tag/v5.0.0-beta.0 is available to test out and verify you're happy with the change.

I'll try it tomorrow and provide feedback here. Unfortunately, I currently don't have access to our Windows build machine. 😞

@jakobvogel
Copy link
Contributor Author

I tried version 5.0.0 with our real product and everything works like a charm, both on macOS (where everything continues to work) and Windows (where credentials have enterprise persistence now). – Apart from the known issue #212, of course, but that does not matter to me too much right now.

So, I am very happy with version 5.0.0. 🎉 I am looking forward to retire our fork of the repository and move back to the official releases. Thanks again @shiftkey! 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change cred.Persist to CRED_PERSIST_ENTERPRISE
3 participants