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

Refactored types into a more organized hierarchy. #153

Merged
merged 3 commits into from
Aug 13, 2024

Conversation

cjbeery24
Copy link

Fixes: #109

Description

Refactored types into a more organized hierarchy.

Changes

  • Organized typescript types

PR Tasks

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

@Egge21M
Copy link
Collaborator

Egge21M commented Jul 18, 2024

I like this alot. Thank you for doing this.

However I think after merging this we need to take this even further for a later major release. As pointed out in #109 some Types are not unambiguous enough. Also thinks like SendResponse (which is not a response) should be renamed IMO.

We can still proceed with merging this change though, to clean things up a little

Copy link
Collaborator

@Egge21M Egge21M left a comment

Choose a reason for hiding this comment

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

LGTM

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.

utACK, you're doing satoshis work 🙏

@Egge21M Egge21M changed the base branch from main to development August 12, 2024 14:04
@Egge21M Egge21M changed the base branch from development to main August 12, 2024 14:04
@Egge21M Egge21M changed the base branch from main to development August 12, 2024 14:05
@Egge21M
Copy link
Collaborator

Egge21M commented Aug 12, 2024

@cjbeery24 could you please rebase this PR to the development branch? I would love to include this with the next release

@cjbeery24
Copy link
Author

@Egge21M Okay, think I did that correctly.

@Egge21M Egge21M merged commit c325f21 into cashubtc:development Aug 13, 2024
1 check passed
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.

Organize request and response models vs. internal models
3 participants