Skip to content

doc: Misc RPC help fixes #17804

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
Merged
laanwj merged 6 commits into from
Feb 5, 2020
Merged

doc: Misc RPC help fixes #17804

laanwj merged 6 commits into from
Feb 5, 2020

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Dec 27, 2019 •
edited
Loading

@fanquake fanquake added the Docs label Dec 27, 2019
@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 27, 2019 •
edited
Loading

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

Conflicts

No conflicts as of last run.

@practicalswift
Copy link
Contributor

practicalswift commented Dec 27, 2019

Concept ACK: nice cleanups!

kristapsk
kristapsk reviewed Dec 27, 2019
View reviewed changes
src/rpc/blockchain.cpp Outdated
Copy link
Contributor

@kristapsk kristapsk Dec 27, 2019

Choose a reason for hiding this comment

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

Maybe change "xxxx" to "true|false" here and elsewhere where it's boolean too, as it is in other places?

Copy link
Member Author

@maflcko maflcko Dec 27, 2019

Choose a reason for hiding this comment

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

Done in #17809

src/rpc/blockchain.cpp Outdated
Copy link
Contributor

@kristapsk kristapsk Dec 27, 2019

Choose a reason for hiding this comment

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

Numeric uses "n" for numbers and "ttt" for timestamps in other places, maybe change others too instead of "xxx"?

Copy link
Member Author

@maflcko maflcko Dec 27, 2019

Choose a reason for hiding this comment

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

Actually, most of the times are denoted with xxx, not ttt. Anyway, this is fixed in #17809

Copy link
Contributor

@promag promag Dec 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Out of curiosity, why is xxxxxx (as well the other "values") necessary? Isn't the type sufficient?

Copy link
Member Author

@maflcko maflcko Dec 28, 2019

Choose a reason for hiding this comment

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

It serves a placeholder for the value in the key-value pair of a json dict

@maflcko maflcko mentioned this pull request Dec 27, 2019
Merged
promag
promag reviewed Dec 27, 2019
View reviewed changes
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Concept ACK.

src/rpc/mining.cpp Outdated
Copy link
Contributor

@promag promag Dec 27, 2019

Choose a reason for hiding this comment

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

👀 this is json-rpc?

Copy link
Member Author

@maflcko maflcko Dec 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, RPC arguments are called this way. Also, most of the RPC results are called this way. This scripted diff adjusts the remaining ones. After #17809 it can be changed to something else with a one-line patch.

@fjahr
Copy link
Contributor

fjahr commented Jan 2, 2020

ACK fa8f302c6354bb41c2e7cbdb3448cc1b91c9a531

Reviewed code and did some selective testing.

@maflcko
Copy link
Member Author

maflcko commented Jan 9, 2020

Rebased due to conflict in scripted-diff. Should be easy to re-ACK

@cvengler
Copy link
Contributor

cvengler commented Jan 24, 2020 •
edited
Loading

This fixes documentation of the following RPCs:

Are they generated automatically? If so how?
Would like to test it (any link to a script?)

@maflcko
Copy link
Member Author

maflcko commented Jan 24, 2020

Are they generated automatically? If so how?

https://github.com/bitcoin-core/bitcoincore.org/tree/master/contrib#rpc-documentation-generation

@maflcko
Copy link
Member Author

maflcko commented Jan 29, 2020

Resolved merge conflict

@laanwj
Copy link
Member

laanwj commented Feb 5, 2020

ACK fa5c662

laanwj added a commit that referenced this pull request Feb 5, 2020
@laanwj
Merge #17804: doc: Misc RPC help fixes
712b7d9 
fa5c662 doc: Use proper RPC help syntax in importmulti (MarcoFalke)
fab6311 doc: Remove duplicate "comment" from listsinceblock RPC help (MarcoFalke)
fa04cd6 doc: Properly document proxy_randomize_credentials as bool in getnetworkinfo (MarcoFalke)
fa9dec7 doc: Fix syntax error (trailing square bracket) in finalizepsbt (MarcoFalke)
faff5a6 doc: Fix syntax error (trailing square bracket) in walletprocesspsbt (MarcoFalke)
fa05459 doc: Add missing "optional" to "long" estimaterawfee RPC help (MarcoFalke)

Pull request description:

  This fixes documentation of the following RPCs:

  * estimaterawfee (hidden)
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/walletprocesspsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/rawtransactions/finalizepsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/network/getnetworkinfo/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/listsinceblock/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/

  <!-- Also, it comes with a scripted diff to normalize whitespace and type names. (Previous attempts: #14601 and #14459)

ACKs for top commit:
  laanwj:
    ACK fa5c662

Tree-SHA512: 5a10956e12f8ce23e93a2ce8bafd6cae759d8a21658f79397e3bfce3e4aabd9658bdbd40acde49323dca958a9befee7166654994208c182dd60f483109621e17
@laanwj laanwj merged commit fa5c662 into bitcoin:master Feb 5, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 9, 2020
@laanwj
Merge bitcoin#17804: doc: Misc RPC help fixes
7ff2546 
fa5c662 doc: Use proper RPC help syntax in importmulti (MarcoFalke)
fab6311 doc: Remove duplicate "comment" from listsinceblock RPC help (MarcoFalke)
fa04cd6 doc: Properly document proxy_randomize_credentials as bool in getnetworkinfo (MarcoFalke)
fa9dec7 doc: Fix syntax error (trailing square bracket) in finalizepsbt (MarcoFalke)
faff5a6 doc: Fix syntax error (trailing square bracket) in walletprocesspsbt (MarcoFalke)
fa05459 doc: Add missing "optional" to "long" estimaterawfee RPC help (MarcoFalke)

Pull request description:

  This fixes documentation of the following RPCs:

  * estimaterawfee (hidden)
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/walletprocesspsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/rawtransactions/finalizepsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/network/getnetworkinfo/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/listsinceblock/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/

  <!-- Also, it comes with a scripted diff to normalize whitespace and type names. (Previous attempts: bitcoin#14601 and bitcoin#14459)

ACKs for top commit:
  laanwj:
    ACK fa5c662

Tree-SHA512: 5a10956e12f8ce23e93a2ce8bafd6cae759d8a21658f79397e3bfce3e4aabd9658bdbd40acde49323dca958a9befee7166654994208c182dd60f483109621e17
@MarkLTZ MarkLTZ mentioned this pull request Apr 4, 2020
Closed
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
@laanwj
Merge bitcoin#17804: doc: Misc RPC help fixes
1cc1121 
fa5c662 doc: Use proper RPC help syntax in importmulti (MarcoFalke)
fab6311 doc: Remove duplicate "comment" from listsinceblock RPC help (MarcoFalke)
fa04cd6 doc: Properly document proxy_randomize_credentials as bool in getnetworkinfo (MarcoFalke)
fa9dec7 doc: Fix syntax error (trailing square bracket) in finalizepsbt (MarcoFalke)
faff5a6 doc: Fix syntax error (trailing square bracket) in walletprocesspsbt (MarcoFalke)
fa05459 doc: Add missing "optional" to "long" estimaterawfee RPC help (MarcoFalke)

Pull request description:

  This fixes documentation of the following RPCs:

  * estimaterawfee (hidden)
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/walletprocesspsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/rawtransactions/finalizepsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/network/getnetworkinfo/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/listsinceblock/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/

  <!-- Also, it comes with a scripted diff to normalize whitespace and type names. (Previous attempts: bitcoin#14601 and bitcoin#14459)

ACKs for top commit:
  laanwj:
    ACK fa5c662

Tree-SHA512: 5a10956e12f8ce23e93a2ce8bafd6cae759d8a21658f79397e3bfce3e4aabd9658bdbd40acde49323dca958a9befee7166654994208c182dd60f483109621e17
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@promag promag promag left review comments

@kristapsk kristapsk kristapsk left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@maflcko @DrahtBot @practicalswift @fjahr @cvengler @laanwj @promag @kristapsk @fanquake