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.

Sign up for GitHub

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

Jump to bottom

[Notify] Unregister method #1182

Merged
merged 13 commits into from
Oct 18, 2023
Merged

[Notify] Unregister method #1182

merged 13 commits into from
Oct 18, 2023

Conversation

flypaper0
Copy link
Contributor

Description

Spcs: WalletConnect/walletconnect-specs#146

Resolves # (issue)

How Has This Been Tested?

Due Dilligence

  • Breaking change
  • Requires a documentation update

@flypaper0 flypaper0 requested a review from llbartekll October 16, 2023 13:12
@flypaper0 flypaper0 temporarily deployed to internal October 16, 2023 13:13 — with GitHub Actions Inactive
@flypaper0 flypaper0 temporarily deployed to internal October 16, 2023 15:04 — with GitHub Actions Inactive
@flypaper0 flypaper0 temporarily deployed to internal October 16, 2023 15:08 — with GitHub Actions Inactive
@flypaper0 flypaper0 force-pushed the feature/notify-unregister branch from fb8d1ba to 6929f0b Compare October 17, 2023 08:51
@flypaper0 flypaper0 temporarily deployed to internal October 17, 2023 08:51 — with GitHub Actions Inactive
@flypaper0 flypaper0 temporarily deployed to internal October 17, 2023 10:52 — with GitHub Actions Inactive
@flypaper0 flypaper0 temporarily deployed to internal October 17, 2023 11:36 — with GitHub Actions Inactive
@flypaper0 flypaper0 temporarily deployed to internal October 18, 2023 04:25 — with GitHub Actions Inactive
llbartekll
llbartekll approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@llbartekll llbartekll left a comment

Choose a reason for hiding this comment

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

looks good, just left few comments

Example/WalletApp/PresentationLayer/Wallet/Notifications/NotificationsInteractor.swift Show resolved Hide resolved
Sources/WalletConnectNotify/Client/Wallet/NotifyWatchAgreementService.swift Outdated Show resolved Hide resolved
Sources/WalletConnectNotify/Client/Wallet/SubscriptionWatcher.swift Show resolved Hide resolved
.../ProtocolEngine/wc_notifyWatchSubscriptions/NotifyWatchSubscriptionsResponseSubscriber.swift Show resolved Hide resolved
Tests/NotifyTests/Mocks/MockNotifyStoring.swift Show resolved Hide resolved
@flypaper0 flypaper0 requested a review from llbartekll October 18, 2023 10:23
@flypaper0 flypaper0 temporarily deployed to internal October 18, 2023 10:26 — with GitHub Actions Inactive
@flypaper0 flypaper0 merged commit fbf5e09 into develop Oct 18, 2023
8 checks passed
@flypaper0 flypaper0 deleted the feature/notify-unregister branch October 18, 2023 10:53
@flypaper0 flypaper0 linked an issue Oct 18, 2023 that may be closed by this pull request
[Notify Beta.1] Logout / unregister (Multi-account support) #1162
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@llbartekll llbartekll Awaiting requested review from llbartekll

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Notify Beta.1] Logout / unregister (Multi-account support)
2 participants
@flypaper0 @llbartekll