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

docs: Consistent type names in RPC help descriptions #14459

Closed
wants to merge 16 commits into from

Conversation

ch4ot1c
Copy link
Contributor

@ch4ot1c ch4ot1c commented Oct 10, 2018

Followup to #14373.

Now, only these appear in description text:

boolean
numeric
string

json array
json array of xs
json object
json object of xs

@ch4ot1c ch4ot1c force-pushed the fix/rpc-descriptions branch 2 times, most recently from c87240c to b96ba90 Compare October 10, 2018 18:37
@ch4ot1c ch4ot1c force-pushed the fix/rpc-descriptions branch 2 times, most recently from b911140 to 060fc25 Compare October 10, 2018 23:52
src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved
@ch4ot1c
Copy link
Contributor Author

ch4ot1c commented Oct 11, 2018

See above comment.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 20, 2018

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14987 (RPCHelpMan: Pass through Result and Examples by MarcoFalke)
  • #14918 (RPCHelpMan: Check default values are given at compile-time by MarcoFalke)
  • #14912 ([WIP] External signer support (e.g. hardware wallet) by Sjors)
  • #14481 (Add P2SH-P2WSH support to listunspent RPC by MeshCollider)
  • #14021 (Import key origin data through descriptors in importmulti by achow101)
  • #12911 (wallet: Show fee in results for signrawtransaction* for segwit inputs by kallewoof)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko
Copy link
Member

maflcko commented Oct 21, 2018

  • importprunedfunds has arguments, but they are not listed in the help string. Mind adding them?
  • uptime takes (and ignores) a dummy argument, whereas it shouldn't. Mind fixing the code to throw when a dummy is provided?

src/rpc/server.cpp Outdated Show resolved Hide resolved
@maflcko
Copy link
Member

maflcko commented Oct 21, 2018

prioritisetransaction has an argument with a space. Mind to replace the space with underscore?
getbalance has an argument name with a opening bracket. Mind to remove those brackets from the name?

src/wallet/rpcdump.cpp Outdated Show resolved Hide resolved
@ch4ot1c ch4ot1c force-pushed the fix/rpc-descriptions branch from 91ed951 to 9b981dd Compare October 21, 2018 04:59
@ch4ot1c
Copy link
Contributor Author

ch4ot1c commented Oct 21, 2018

prioritisetransaction and getbalance changes are already in the PR.

Edit: All above issues by MarcoFalke have been resolved by the latest code having transitioned to RPCHelpMan.

src/zmq/zmqrpc.cpp Outdated Show resolved Hide resolved
@ch4ot1c ch4ot1c force-pushed the fix/rpc-descriptions branch 3 times, most recently from a7bae21 to e300778 Compare October 22, 2018 01:09
-BEGIN VERIFY SCRIPT-

git grep -l "Must be one of\\\n" src | xargs sed -i "s/Must be one of\\\n/Must be one of:\\\n/g"

-END VERIFY SCRIPT-
-BEGIN VERIFY SCRIPT-

git grep -l "[aA]n array" src/rpc src/wallet | xargs sed -i "s/an array/a json array/g"

git grep -l "[aA]n array" src/rpc src/wallet | xargs sed -i "s/An array/A json array/g"

-END VERIFY SCRIPT-
…object'

-BEGIN VERIFY SCRIPT-

git grep -l "pair not an object" test/functional/rpc_rawtransaction.py | xargs sed -i "s/pair not an object/pair not a json object/g"

-END VERIFY SCRIPT-
@ch4ot1c ch4ot1c force-pushed the fix/rpc-descriptions branch from 6046c78 to ff7329f Compare December 28, 2018 19:23
@ch4ot1c
Copy link
Contributor Author

ch4ot1c commented Dec 28, 2018

Rebased and ready for review @MarcoFalke. Let me know how you'd like this squashed.

…lockchain.cpp

-BEGIN VERIFY SCRIPT-

git grep -l "of numeric" src/rpc/blockchain.cpp | xargs sed -i "s/of numeric/of numerics/g"

-END VERIFY SCRIPT-
… src/rpc, src/wallet

-BEGIN VERIFY SCRIPT-

git grep -l "(object) " src/wallet src/rpc | xargs sed -i "s/(object) /(json object) /g"

-END VERIFY SCRIPT-
@fanquake fanquake changed the title More RPC help description fixes docs: More RPC help description fixes Dec 29, 2018
@ch4ot1c ch4ot1c changed the title docs: More RPC help description fixes docs: Consistent type names in RPC help descriptions Dec 29, 2018
@laanwj
Copy link
Member

laanwj commented Jan 14, 2019

Concept ACK.

Though I think the jargon switch to numeric loses information.

The code, in practice, makes a difference between numeric (in general) and integer. For example where get_int is used, only an integer will be accepted. Not a decimal value with a point.

@DrahtBot
Copy link
Contributor

Needs rebase

@fanquake fanquake closed this Jun 17, 2019
laanwj added a commit that referenced this pull request Feb 5, 2020
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
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 9, 2020
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
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Feb 17, 2020
…Normalize type names

fad027f scripted-diff: Add missing spaces in RPCResult, Fix type names (MarcoFalke)

Pull request description:

  This makes the rendered diff smaller when the RPCResult is machine generated later on (Previous attempts: bitcoin#14601 and bitcoin#14459)

ACKs for top commit:
  Sjors:
    ACK fad027f

Tree-SHA512: 48afd571b1cd349ca0b29bb444c1c7cda657e07dd96c610d479f931ccd938186aec98e533d0552b5b10afc9a3d7b911359260a49448e8e1106e3647b2c71f3ba
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
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
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 29, 2021
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from bitcoin#14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 3, 2021
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from bitcoin#14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 5, 2021
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from bitcoin#14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 8, 2021
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from bitcoin#14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 9, 2021
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from bitcoin#14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 11, 2021
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from bitcoin#14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
5tefan pushed a commit to 5tefan/dash that referenced this pull request Aug 12, 2021
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from bitcoin#14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
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.

7 participants