Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Nov 13, 2018

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 #14459 (More RPC help description fixes by ch4ot1c)

@maflcko
Copy link
Member Author

maflcko commented Nov 13, 2018

cc @ch4ot1c

@maflcko maflcko force-pushed the Mf1811-rpcNameArguments branch from 5385e4e to fad8314 Compare November 13, 2018 19:20
@maflcko maflcko force-pushed the Mf1811-rpcNameArguments branch from fad8314 to fa0815c Compare November 13, 2018 19:24
@meshcollider
Copy link
Contributor

utACK fa0815c

@practicalswift
Copy link
Contributor

Concept ACK

Nice cleanup!

@laanwj
Copy link
Member

laanwj commented Nov 13, 2018

Makes sense, though I guess the idea eventually would be to use the just-merged RPCHelpMan here?

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

Might make sense to split them out to a commit that goes in before the src changes to make this clear.

@maflcko
Copy link
Member Author

maflcko commented Nov 13, 2018

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

@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14411 ([wallet] Restore ability to list incoming transactions by label by ryanofsky)
  • #13756 (wallet: "avoid_reuse" wallet flag for improved privacy by kallewoof)
  • #12674 (RPC: Support addnode onetry without making the connection priviliged by luke-jr)
  • #10593 (Relax punishment for peers relaying invalid blocks and headers by luke-jr)

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.

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

utACK fa0815c

@maflcko maflcko merged commit fa0815c into bitcoin:master Nov 13, 2018
maflcko pushed a commit that referenced this pull request Nov 13, 2018
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 #14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
@maflcko maflcko deleted the Mf1811-rpcNameArguments branch November 13, 2018 21:54
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 13, 2020
Summary:
fa0815c300 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 #14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c

Backport of Core [[bitcoin/bitcoin#14720 | PR14720]]
bitcoin/bitcoin#14720

Test Plan:
  ninja check
  ninja check-functional

  ./bitcoind
  ./bitcoin-cli help waitforblockheight
  ./bitcoin-cli help getblockheader
  ./bitcoin-cli help pruneblockchain
  ./bitcoin-cli help getchaintxstats
  ./bitcoin-cli help scantxoutset
  ./bitcoin-cli help prioritisetransaction
  ./bitcoin-cli help getblocktemplate
  ./bitcoin-cli help addnode
  ./bitcoin-cli help disconnectnode
  ./bitcoin-cli help setban
  ./bitcoin-cli help setnetworkactive
  ./bitcoin-cli help uptime
  ./bitcoin-cli help importprunedfunds
  ./bitcoin-cli help importmulti
  ./bitcoin-cli help getbalance
  ./bitcoin-cli help listtransactions
Verify help text

  ./bitcoin-cli uptime <anything>
Verify this fails with error code -1 and then displays the help message.

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5281
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 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