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

Modify command line help to show support for BIP21 URIs #752

Merged
merged 1 commit into from
Feb 11, 2024

Conversation

hernanmarino
Copy link
Contributor

@hernanmarino hernanmarino commented Sep 8, 2023

While reviewing a different PR (see #742 ) hebasto suggested that the help for bitcoin-qt should be updated to reflect the fact that bitcoin-qt supports an optional BIP21 URI parameter.

Since this reflects actual behaviour of bitcoin-qt and is independent of whether or not the other PR gets merged, I created this simple PR to fix the help message.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 8, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK kristapsk, pablomartin4btc, hebasto
Concept ACK RandyMcMillan
Stale ACK jarolrod

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

@RandyMcMillan
Copy link
Contributor

concept ACK

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK b45658a

src/qt/utilitydialog.cpp Outdated Show resolved Hide resolved
src/qt/utilitydialog.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

tested ACK.

image

Added some obs on @luke-jr's comments.

src/qt/utilitydialog.cpp Outdated Show resolved Hide resolved
src/qt/utilitydialog.cpp Outdated Show resolved Hide resolved
src/qt/utilitydialog.cpp Outdated Show resolved Hide resolved
@hernanmarino
Copy link
Contributor Author

Updated to address @luke-jr and @pablomartin4btc

Copy link
Contributor

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

re ACK

src/qt/utilitydialog.cpp Outdated Show resolved Hide resolved
src/qt/utilitydialog.cpp Outdated Show resolved Hide resolved
@hernanmarino
Copy link
Contributor Author

Fixed the 2 typos mentioned by @luke-jr and @kristapsk . Also rebased for CI.

@DrahtBot DrahtBot removed the CI failed label Feb 1, 2024
Copy link
Contributor

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

ack ede5014

Copy link
Contributor

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

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

utACK ede5014

Copy link
Contributor

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

lgtm, re ACK ede5014

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK ede5014.

@hebasto hebasto merged commit 9e68a82 into bitcoin-core:master Feb 11, 2024
16 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.

9 participants