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

Add -l/--label-change option to sendpayment.py to automatically label change address #1468

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

kristapsk
Copy link
Member

Also added typehints to direct_send().

@kristapsk kristapsk force-pushed the sendpayment-label branch 4 times, most recently from 4dc08bf to 8577917 Compare April 6, 2023 22:04
@kristapsk
Copy link
Member Author

If nobody wants to review / test this, I plan to merge this soon, should not affect any existing functionality.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 11, 2023

Looking now.

jmclient/jmclient/taker.py Outdated Show resolved Hide resolved
@AdamISZ
Copy link
Member

AdamISZ commented Apr 11, 2023

I agree with the rest apart from the above two comments.

However, as with all such UI type changes, it makes sense to include an adaptation to both Qt and, especially, to the RPC interface, to allow clients to use them.

@kristapsk
Copy link
Member Author

as with all such UI type changes, it makes sense to include an adaptation to both Qt and, especially, to the RPC interface

Agree, but that can be done in a follow-up PRs, created separate issues for RPC API and Qt GUI.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 14, 2023

Will test 34c0c45

@AdamISZ
Copy link
Member

AdamISZ commented Apr 14, 2023

I have tested both --label-change="XXX" and -l YYY options to sendpayment.py using both direct send -N 0 and normal coinjoin defaults, on both signet and mainnet.

tACK 34c0c45

@kristapsk kristapsk merged commit df17708 into JoinMarket-Org:master Apr 14, 2023
@kristapsk kristapsk deleted the sendpayment-label branch April 14, 2023 01:26
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.

2 participants