Skip to content

improv: simplify create wallet code #284

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

Merged
merged 5 commits into from
Apr 21, 2025

Conversation

r1b2ns
Copy link
Collaborator

@r1b2ns r1b2ns commented Apr 11, 2025

Description

Refactoring connection creation in BitcoinKit for cleaner code in BDKService file.

Notes to the reviewers

Tiny PR to simplify the code readability.

Changelog notice

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • UI changes tested on small, medium, and large devices to ensure layout consistency

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@r1b2ns r1b2ns closed this Apr 11, 2025
@r1b2ns r1b2ns reopened this Apr 11, 2025
@r1b2ns r1b2ns force-pushed the improv/create-wallet-bdkservice branch from 8fa0f73 to 2db9a9e Compare April 12, 2025 20:21
@r1b2ns r1b2ns marked this pull request as ready for review April 12, 2025 20:23
@reez
Copy link
Collaborator

reez commented Apr 15, 2025

Hey thanks for this! I will have a chance to review it next week

@r1b2ns
Copy link
Collaborator Author

r1b2ns commented Apr 15, 2025

Tks Reez

@reez reez requested a review from Copilot April 21, 2025 16:42
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the wallet creation code in BDKService to simplify connection initialization and URL retrieval, improving overall code readability. Key changes include:

  • Moving connection creation logic into a dedicated extension in Connection+Extensions.swift.
  • Replacing repetitive switch-case constructs for Esplora URL retrieval with a computed property on Network.
  • Streamlining the createWallet methods across different wallet creation flows.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
BDKSwiftExampleWallet/Utilities/Constants.swift Added an extension to Network for retrieving the Esplora URL based on network type.
BDKSwiftExampleWallet/Service/BDK Service/BDKService.swift Simplified wallet creation by removing duplicate switch-case logic and utilizing the new connection creation helper.
BDKSwiftExampleWallet/Extensions/BDK+Extensions/Connection+Extensions.swift Introduced a helper method for connection creation that encapsulates file management tasks.
Files not reviewed (1)
  • BDKSwiftExampleWallet.xcodeproj/project.pbxproj: Language not supported

@reez
Copy link
Collaborator

reez commented Apr 21, 2025

great improvement, I like it

@reez reez merged commit 11eefd1 into bitcoindevkit:main Apr 21, 2025
1 check passed
@r1b2ns r1b2ns deleted the improv/create-wallet-bdkservice branch April 22, 2025 09:53
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