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

refactor: create wallet namespace #1051

Merged
merged 3 commits into from
Apr 3, 2024

Conversation

PhilippeR26
Copy link
Collaborator

@PhilippeR26 PhilippeR26 commented Mar 28, 2024

Motivation and Resolution

Wallet API functions have to be imported one by one from "starknet"
Create a wallet namespace is solving this problem, and will group all methods related to this subject.

Usage related changes

image

Development related changes

connect.ts renamed wallet.ts

Checklist:

  • Performed a self-review of the code
  • Rebased to the last commit of the target branch (or merged it into my branch)
  • All tests are passing

src/index.ts Outdated
@@ -43,6 +43,7 @@ export * from './utils/calldata/enum';
export * from './utils/contract';
export * from './utils/events';
export * from './utils/transactionReceipt';
export * as wallet from './wallet/wallet';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know there was a discussion about this on Discord, but if we have a better idea not to have wallet/wallet ? I know naming is the hardest thing in development and I'm nitpicking xD
cc @tabaktoni

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you prefer a file called walletAPI.ts ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

One possibility is to use export * as wallet from './wallet' in src/wallet/index.ts in place of export * from './connect' instead of removing it (could even keep the connect file name).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I went back to connect.ts, keeping export * as wallet.

Copy link
Collaborator

@tabaktoni tabaktoni left a comment

Choose a reason for hiding this comment

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

Ok, lgtm

@tabaktoni tabaktoni merged commit 173d7e7 into starknet-io:next-version Apr 3, 2024
1 of 2 checks passed
@tabaktoni tabaktoni mentioned this pull request Apr 3, 2024
6 tasks
penovicp pushed a commit that referenced this pull request Apr 3, 2024
* refactor: create wallet namespace

* fix: change name to connect.ts

---------

Co-authored-by: Toni Tabak <tabaktoni@gmail.com>
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