Skip to content
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

Add initial storage manager #14

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

theosirian
Copy link
Contributor

No description provided.

@theosirian theosirian force-pushed the feat/camv-531-sprucekit-sdk-storage-manager-kotlin branch from 4dbeb00 to a61307a Compare June 12, 2024 15:04
@cobward cobward requested review from cobward and removed request for CharlesShuller August 1, 2024 14:12
val dataStore: DataStore<Preferences> = store(context, "default")

companion object {
private const val FILENAME_PREFIX = "datastore_"
Copy link
Contributor

Choose a reason for hiding this comment

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

For the prefix and the encryption key, could we use something sprucekit specific to avoid name clashes when someone imports this library into their application? Maybe:

  • sprucekit/datastore/ directories to store the files in?
  • sprucekit-datastore for the encryption key?

Copy link
Contributor

Choose a reason for hiding this comment

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

@todd-spruceid WDYT doing something similar in mobile-sdk-swift for consistency?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be inclined to suggest just sprucekit for the directory, but regardless, it probably makes sense to do the same in -swift, mostly for ease of maintenance.

Copy link
Contributor

@cobward cobward left a comment

Choose a reason for hiding this comment

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

Some conflicts need resolving, otherwise LGTM.

Copy link
Contributor

@todd-spruceid todd-spruceid left a comment

Choose a reason for hiding this comment

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

There's a list() being added to the protocol (see spruceid/mobile-sdk-rs#24) which will affect this code later.

@rschulman rschulman force-pushed the feat/camv-531-sprucekit-sdk-storage-manager-kotlin branch from e591d89 to c3a10d5 Compare September 11, 2024 21:59
@rschulman
Copy link
Contributor

Rebased onto main and fixed one reference to wallet.sdk.

@rschulman rschulman requested review from sbihel, cobward and todd-spruceid and removed request for jszersze September 11, 2024 22:01
@rschulman rschulman merged commit 236f0a0 into main Sep 16, 2024
2 checks passed
@rschulman rschulman deleted the feat/camv-531-sprucekit-sdk-storage-manager-kotlin branch September 16, 2024 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants