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

Chore/bindings: cleanup is alive 2.0 #1541

Merged
merged 73 commits into from
Dec 13, 2023

Conversation

kwek20
Copy link
Contributor

@kwek20 kwek20 commented Oct 30, 2023

Features:

  • Align error handling in node (all try/catch in x_method_handler
  • Align async on functions
  • Adds json error return instead of string
  • Simplified bindings code for error returns

TODO:

  • Figure out how to not clone client
  • Align secretmanager with the wallet and client code (it doesnt have option, but gets returned as Arc<RwLock<>> so cant unwrap unless cloning nonsense, so cant use the binding_glue! will be done after secret rewrite
  • Align get_client and get_secret_manager in wallet

maybe more?

Closes #699

Replaces #1119

@kwek20 kwek20 marked this pull request as draft October 30, 2023 22:16
Copy link
Member

@thibault-martinez thibault-martinez left a comment

Choose a reason for hiding this comment

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

Need to solve conflicts and see what still makes sense since we merged the NAPI stuff

@kwek20 kwek20 force-pushed the chore/bindings-cleanup-is-alive branch from 0005505 to 72a5c32 Compare November 12, 2023 16:09
@kwek20
Copy link
Contributor Author

kwek20 commented Dec 5, 2023

Checked out suggestions form @DaughterOfMars, but decided it wasnt worth it now. new issue: #1746

Updated description TODOs

@Alex6323
Copy link
Contributor

Alex6323 commented Dec 8, 2023

merge?

@kwek20 kwek20 enabled auto-merge (squash) December 12, 2023 12:23
@kwek20 kwek20 merged commit 51011bd into iotaledger:2.0 Dec 13, 2023
30 of 36 checks 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.

5 participants