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

util: improve bitcoin-wallet exit codes #26067

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Sep 12, 2022

Refactors bitcoin-wallet so that it doesn't return a non-zero exit code by default, and makes the option handling more inline with the other binaries. i.e outputting Error: too few parameters if you don't pass any options.

Fixing this means we can check the process output in gen-manpages.py; which addresses the remaining review comment from #24263.

Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

utACK

@maflcko maflcko merged commit d76a423 into bitcoin:master Sep 20, 2022
@maflcko maflcko deleted the 2209-wallet-exit-🛶 branch September 20, 2022 07:57
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 20, 2022
@@ -36,7 +36,7 @@
for relpath in BINARIES:
abspath = os.path.join(builddir, relpath)
try:
r = subprocess.run([abspath, '--version'], stdout=subprocess.PIPE, universal_newlines=True)
r = subprocess.run([abspath, "--version"], stdout=subprocess.PIPE, check=True, universal_newlines=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change single quote to double quote? Is it a coding style in the Bitcoin Core?

@Riahiamirreza
Copy link
Contributor

As far as I could grasp the codebase, ACK.

@bitcoin bitcoin locked and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants