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

Remove send_asset procedure from the basic wallet #829

Merged
merged 3 commits into from
Sep 20, 2024
Merged

Remove send_asset procedure from the basic wallet #829

merged 3 commits into from
Sep 20, 2024

Conversation

Fumuran
Copy link
Contributor

@Fumuran Fumuran commented Aug 13, 2024 •
edited
Loading

This PR removes the send_asset procedure from the basic wallet.
To send a note with some assets create_note and move_asset_to_note procedures should be used instead.

@Fumuran Fumuran linked an issue Aug 13, 2024 that may be closed by this pull request
Remove send_asset procedure from the basic wallet. #821
Closed
@Fumuran Fumuran requested review from bobbinth and igamigo August 13, 2024 09:52
@bobbinth
Copy link
Contributor

We'll need to rebase this one after #826 is merged.

@Fumuran Fumuran force-pushed the andrew-remove-send-asset branch from 4ebc051 to 92ba9ef Compare August 23, 2024 15:07
bobbinth
bobbinth approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left a few small comments inline.

Also, as mentioned in one comment, I think this should probably go into v0.6 release. Maybe even we should merge it after #803.

@Fumuran Fumuran force-pushed the andrew-remove-send-asset branch from 92ba9ef to 3955d97 Compare August 26, 2024 10:37
@Fumuran Fumuran added this to the v0.6 milestone Aug 29, 2024
@Fumuran Fumuran force-pushed the andrew-remove-send-asset branch 2 times, most recently from f1e0231 to 994c123 Compare September 19, 2024 17:27
@bobbinth
Copy link
Contributor

@igamigo - this will probably be a breaking change for the client. But I think we should be able to update the client even now to use create_note and move_asset_to_note instead of send_asset. So, maybe we update the client first, and then merge this PR?

@igamigo
Copy link
Collaborator

igamigo commented Sep 20, 2024

@igamigo - this will probably be a breaking change for the client. But I think we should be able to update the client even now to use create_note and move_asset_to_note instead of send_asset. So, maybe we update the client first, and then merge this PR?

Makes sense! Will take a look first thing tomorrow, should be a simple change.

@Fumuran Fumuran force-pushed the andrew-remove-send-asset branch from 994c123 to d8ad2d2 Compare September 20, 2024 17:35
@bobbinth bobbinth merged commit 14d7365 into next Sep 20, 2024
8 checks passed
@bobbinth bobbinth deleted the andrew-remove-send-asset branch September 20, 2024 18:09
This was referenced Sep 20, 2024
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobbinth bobbinth bobbinth approved these changes

@igamigo igamigo Awaiting requested review from igamigo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.6
Development

Successfully merging this pull request may close these issues.

Remove send_asset procedure from the basic wallet.
3 participants
@Fumuran @bobbinth @igamigo