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

Naming Methods Mint/Melt #145

Merged
merged 5 commits into from
Jul 16, 2024
Merged

Naming Methods Mint/Melt #145

merged 5 commits into from
Jul 16, 2024

Conversation

Egge21M
Copy link
Collaborator

@Egge21M Egge21M commented Jul 5, 2024

Fixes: #143

Description

Feedback from the hackday was that naming of some user facing methods of CashuWallet were not clearly named. This PR addresses this issue. It also changes the names of corresponding CashuMint methods to reflect the protocol spec more closely.

Changes

  • Changed CashuWallet.mintQuote to CashuWallet.createMintQuote
  • Changed CashuMint.mintQuote to CashuMint.postMintQuote
  • Changed CashuWallet.getmintQuote to CashuWallet.checkMintQuote
  • ... The same for meltQuote and getMeltQuote

PR Tasks

  • Open PR
  • run npm run test --> no failing unit tests
  • run npm run format

@Egge21M Egge21M requested a review from gandlafbtc July 5, 2024 09:28
@Egge21M Egge21M changed the base branch from main to development July 5, 2024 09:28
gandlafbtc
gandlafbtc approved these changes Jul 16, 2024
View reviewed changes
Copy link
Collaborator

@gandlafbtc gandlafbtc left a comment

Choose a reason for hiding this comment

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

LGTM

@callebtc
Copy link
Contributor

callebtc commented Jul 16, 2024 •
edited
Loading

Any reason why you call one createMintQuote and the other postMintQuote if both are the same thing? One name for both would be better. I'd suggest replacing all with createMintQuote.

callebtc
callebtc requested changes Jul 16, 2024
View reviewed changes
src/CashuWallet.ts Outdated Show resolved Hide resolved
callebtc
callebtc approved these changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@callebtc callebtc left a comment

Choose a reason for hiding this comment

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

LGTM

@callebtc callebtc merged commit 55dc4fa into cashubtc:development Jul 16, 2024
1 check passed
Egge21M pushed a commit to Egge21M/cashu-ts that referenced this pull request Jul 16, 2024
@callebtc @Egge21M
Merge pull request cashubtc#145 from Egge21M/naming
7f74da2 
Naming Methods Mint/Melt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gandlafbtc gandlafbtc gandlafbtc approved these changes

@callebtc callebtc callebtc approved these changes

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

Successfully merging this pull request may close these issues.

V1 naming
3 participants
@Egge21M @callebtc @gandlafbtc