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

User defaults group migration #1240

Merged
merged 12 commits into from
Dec 1, 2023
Merged

Conversation

llbartekll
Copy link
Contributor

Description

Resolves # (issue)

How Has This Been Tested?

Due Dilligence

  • Breaking change
  • Requires a documentation update

@llbartekll llbartekll requested a review from flypaper0 November 23, 2023 13:42
@llbartekll llbartekll marked this pull request as draft November 23, 2023 13:42
@llbartekll llbartekll marked this pull request as ready for review November 28, 2023 14:40
@@ -8,13 +8,20 @@ public class Networking {
}

public static var interactor: NetworkingInteractor = {
guard let _ = Networking.config else {
Copy link
Contributor

Choose a reason for hiding this comment

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

It was empty? lol

}()

public static var groupIdentifier: String {
guard let config = Networking.config else {
Copy link
Contributor

Choose a reason for hiding this comment

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

we're copy and pasting fatal error. May be create private static var config: Config and throw fatal error only there?


let keychainStorage = KeychainStorage(serviceIdentifier: "com.walletconnect.sdk")
let keyValueStorage = UserDefaults.standard
let keyValueStorage = UserDefaults(suiteName: groupIdentifier) ?? UserDefaults.standard
Copy link
Contributor

Choose a reason for hiding this comment

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

let's consider Factory?

@llbartekll llbartekll merged commit ef8dab0 into develop Dec 1, 2023
8 checks passed
@llbartekll llbartekll deleted the user-defaults-group-migration branch December 1, 2023 10:57
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.

2 participants