[refactor-prompt] Improve send & sendmany #797
Conversation
Merge CoZ refactor-prompt into jseagrave21 refactor-prompt
- add CommandShow and CommandSearch to prompt
revert changes
Merge CoZ refactor-prompt with jseagrave21 refactor-prompt
Merge CoZ refactor-prompt into jseagrave21 refactor-prompt
Merge CoZ refactor-prompt into jseagrave21 refactor-prompt
Merge CoZ refactor-prompt into jseagrave21 refactor-prompt
Merge CoZ refactor-prompt into jseagrave21 refactor-prompt
Merge CoZ refactor-prompt into jseagrave21 refactor-prompt
- add support for a KeyboardInterrupt during tx creation in `sendmany` - add printout for fee being sent in `send` - replace `return None` with `return` to keep with the rest of the refactoring - fix some outputs
- add test for KeyboardInterrupt - add verification of user output during tests
- remove unnecessary code - un-indent lines to ensure the tests are completed
LysanderGG
left a comment
LysanderGG
left a comment
There was a problem hiding this comment.
Overall looks good to me.
When you rename a file, please use git mv in order to keep the history.
I couldn't review the tests :/
| logger.debug("invalid fee") | ||
| return None | ||
| return | ||
| print(f"Sending with fee: {fee.ToString()}") |
There was a problem hiding this comment.
Is it necessary to print when the fee is 0?
There was a problem hiding this comment.
That is what the original code did and how sendmany works. I was adding it back in. I think it is a good reminder, especially now that priority is a big deal. So they can cancel still if they want to add priority.
|
@LysanderGG you and @ixje have both mentioned using that. Usually when I submit a PR I use GitHub via the web browser, which is how I did it this time too. I would like to close this PR and try it again so you can review. Can I use the web browser and command line? Also, how exactly would I do it from command line? |
|
I don't think you can do it through the github UI. See https://help.github.com/articles/renaming-a-file-using-the-command-line/ for more details. |
|
Okay I will recreate this PR and try to use the above help to rename |
2 participants
What current issue(s) does this address, or what feature is it adding?
sendmanysendreturn Nonewithreturnwhich is in line with other refactoringtest_send_command.pyastest_send_commands.pyto match othersHow did you solve this problem?
Trial and Error
How did you make sure your solution works?
Manual Testing and unittest
Are there any special changes in the code that we should be aware of?
No
Please check the following, if applicable:
make lint?make test?